Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
As part of #1830588: [META] remove drupal_set_title() and drupal_get_title(), all calls to drupal_set_title() in core/authorize.php and core/includes/authorize.inc should be converted to use #title in a render array.
Suggested test process
- Enable update manager module
- Go to the install new module page, admin/modules/install
- Install a module with a Drupal 8 version, e.g. devel or config_inspector
- If your page just reloads on submitting the form, see #2042447: Install a module user interface does not install modules (or themes)
There is a potential conflict with #2169765: Redesign update.php to be more consistent with the installation process, but I don't think this will affect the changes in #7.
Comment | File | Size | Author |
---|---|---|---|
#7 | drupal-2192653-set_title-authorize-7.patch | 4.33 KB | ianthomas_uk |
#2 | 2192653-1.patch | 2.34 KB | divesh.kumar |
Comments
Comment #1
divesh.kumar CreditAttribution: divesh.kumar commentedI'll take on Authorization files.
Comment #2
divesh.kumar CreditAttribution: divesh.kumar commentedMade required changes.
Comment #4
sidharthapHere it fails simple test.$title = $_SESSION['authorize_operation']['page_title']; Title is being render before this $title rendered to page.
Comment #5
divesh.kumar CreditAttribution: divesh.kumar commentedHi sidharthap,
All of the instance of drupal_set_title are getting change without failing any test. However when I remove this one it fails the test.
Any help or pointers on this?
Comment #6
ianthomas_uk#2060401: Remove useless "early render" in authorize.php may be relevant
Comment #7
ianthomas_ukI started the attached patch from scratch, but it ended up broadly similar to #2.
#2 didn't replace the title it removed from authorize_access_denied_page(). That was presumably because it was inside a function, so you couldn't just use a local variable like some of the others. That function didn't seem to add much value, so I've just copied its contents inline (wrapping the 'Access Denied' title inside t() in the process).
There was a bit of a weird dance going on with $_SESSION['authorize_operation']['page_title'], because all of $_SESSION['authorize_operation'] gets unset, which is why drupal_set_title needed to be called from authorize_run_operation() and also why the title has to be restored in update_authorize_update_batch_finished. By changing the title to $_SESSION['authorize_page_title'] it won't get unset, so none of that is needed (the title will just be picked up from $_SESSION in authorize.php).
Comment #8
awochna CreditAttribution: awochna commentedTested patch (drupal-2192653-set_title-authorize-7.patch) and everything works as expected.
Comment #9
Wim LeersI don't have a local FTP server going, so I can't use the update manager completely. However, I do get to see the previous "Update manager" title, so AFAICT all is well.
RTBC +1
Comment #10
webchickCommitted and pushed to 8.x. Thanks!
Comment #11
ianthomas_ukThis doesn't seem to have been pushed.
Comment #12
webchickSorry, Drupal.org hates me. Pushed now.
Comment #14
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedRemoving those broke the page title in some cases - adding it back in #2547667: Fix broken page title Update Manager (authorize.php). It is admittedly confusing why it needs to be there since it's also set elsewhere, but I think the other places only set it under a limited set of conditions or something.