Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update.php: Do not add language prefix to "Skip backup" and "Cancel" link #6832

Closed
indigoxela opened this issue Jan 17, 2025 · 6 comments · Fixed by backdrop/backdrop#4999
Closed

Comments

@indigoxela
Copy link
Member

indigoxela commented Jan 17, 2025

Description of the bug

This only affects multilingual sites, which have a language prefix set for the default language.

Steps To Reproduce

  1. Enable the language and locale module
  2. Configure the language to use a prefix - also the default language
  3. Run an update
  4. When coming to the "Pre-update backup" step, click "Skip backup"

skip backup link

Actual behavior

You end up with a 404.

The requested page "/en/core/update.php?op=selection&token=4_jjblabla..." could not be found.

Expected behavior

Getting to the next step.

Additional information

  • Backdrop CMS version: 1.30.0 (or dev)

This happens, because url() adds the language prefix to the link.

The function chain for the skip link is:

theme_link() -> l() -> url()

And the url() function considers the language, but our default page to update can't handle that.

The quick workaround is to copy the link and strip the language prefix manually, to get to the next step. Doesn't seem ideal. 😉

BTW: the same is true for the "Cancel" link on the next page ("Review updates") - 404.

cancel link

@indigoxela
Copy link
Member Author

indigoxela commented Jan 17, 2025

My suggestion would be to use a more low-level approach for creating links in update.php.
Similar to what we did in #5092

There's no need for aliases, no need for sanitizing, ... But maybe I'm missing something.

@indigoxela indigoxela changed the title Do not add language prefix to "Skip backup" link update.php: Do not add language prefix to "Skip backup" and "Cancel" link Jan 17, 2025
@indigoxela
Copy link
Member Author

indigoxela commented Jan 21, 2025

A PR is available for testing and review.

I don't think, it's possible to do testing only in the sandbox, as there needs to be a trigger for an update.

For local testing I'd recommend a custom module, where you uncomment the update hook after you installed it.

foobar.info:

name = Foobar
description = Test update stuff
type = module
backdrop = 1.x

foobar.install:

<?php

/**
 * Trigger update.
 */
//function foobar_update_1000() {}

^^ after intalling this module, uncomment the update hook and visit admin/reports/status.

@olafgrabienski
Copy link

Issue confirmed, nice catch!

I've tested locally with the Foobar module (needs also a .module file, btw), and the PR works for me. I've also checked if the Skip link continues to work on a multilingual site without prefix for the default language, which is the case.

@argiepiano
Copy link

Also reproduced, also confirmed the patch makes sense, code LGTM

@quicksketch
Copy link
Member

Thank you @indigoxela, @olafgrabienski, and @argiepiano! I merged backdrop/backdrop#4999 into 1.x and 1.30.x.

I have not checked, but restore.php probably has the same problems with links.

@indigoxela
Copy link
Member Author

I have not checked, but restore.php probably has the same problems with links.

The links in restore.php are OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants