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.
Subtask of #1830588: [META] remove drupal_set_title() and drupal_get_title()
Problem/Motivation
Using procedural drupal_set_title() inside controller class is not encouraged.
Proposed resolution
Replace drupal_set_title() with #title in page return array.
Remaining tasks
Issue patch
User interface changes
Refer parent issue at #1830588: [META] remove drupal_set_title() and drupal_get_title()
API changes
Refer parent issue at #1830588: [META] remove drupal_set_title() and drupal_get_title()
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#30 | 2102369-block-title-30.patch | 5.56 KB | JeroenT |
#28 | 2102369-block-title-28.patch | 5.58 KB | JeroenT |
#26 | interdiff.txt | 1.51 KB | ACF |
#26 | 2102369-block-title-26.patch | 5.56 KB | ACF |
#22 | 2102369-block-title-22.patch | 5.55 KB | rteijeiro |
Comments
Comment #1
vijaycs85Initial patch for block & custom block module. We still have 2 instance of drupal_set_title in these modules. Both of them on EntityFormContollers. Not very clear how to make them work.
1. CustomBlockTypeListController->render()
2. CustomBlockFormController->form().
Other than title change, injected t() - called on title.
Comment #2
tim.plunkettA lot of that isn't needed.
Comment #3
dawehnerge FORM!
Comment #5
vijaycs85Updated the method name here...
Comment #6
dawehnerHa
Comment #8
tim.plunkettThese need to be left in until #2047633: Move definition of menu links to hook_menu_link_defaults(), decouple key name from path, and make 'parent' explicit goes in
Comment #9
-enzo- CreditAttribution: -enzo- commentedI re-roll the patch to remove the deletion of menu entries based in #8 by @tim.plunkett
Comment #10
tim.plunkettThis looks good to me.
Comment #12
tim.plunkett#9: drupal8.block-module.2102369-9.patch queued for re-testing.
Comment #13
JulienD CreditAttribution: JulienD commentedThe patch works fine. Blocks titles are still displayed in the menu link or on the admin block pages.
Comment #14
JulienD CreditAttribution: JulienD commentedAs said in #1 there are 2 left drupal_set_title function.
Comment #15
vijaycs85sorry for missing them and thanks for review @JulienD. Here is a patch that fix them as well. I haven't tested it locally yet. Still submitting to get it validated by @tim.plunkett or @dawehner for the approach.
Comment #17
JulienD CreditAttribution: JulienD commentedHi @vijaycs85,
I think there is too much things in your patch (39 files changed, 428 insertions, 360 deletions).
Comment #18
vijaycs85Sorry wrong commit range. This should work.
Comment #20
dawehner#18: 2102369-block-title-18.patch queued for re-testing.
Comment #21
dawehnerIt seems odd to talk about forms even we are on a listing.
If you remove the drupal_set_title() call all you have is return parent::render() so it can be dropped.
Comment #22
rteijeiro CreditAttribution: rteijeiro commentedRe-rolled and applied @dawehner suggestions on #21.
Comment #23
dawehnerThis seems fine for me now.
Comment #24
alexpottHow about getAddFormTitle? The function doc block and name are not consistent. Also can't we just return the title on the form array?
Comment #25
tim.plunkettAlso, this is way bigger than block module. Can we either reduce the scope or retitle the issue?
Comment #26
ACF CreditAttribution: ACF commentedChanged the title to getAddFormTitle. Can't add the page title directly to the form as the form is called both for /block/add and /block/add/{custom_block_type} but they have two different page titles and no way as far as I can see of distushing the two different times the form is called particularly if the custom_block_type is basic.
Comment #27
benjy CreditAttribution: benjy commentedNeeds reroll.
Comment #28
JeroenTRerolled patch #26.
Comment #29
vijaycs85Minor: Empty space...
Comment #30
JeroenTWoops. Fixed the whitespace.
Patch attached.
Comment #31
dawehnerBack to RTBC
Comment #33
JeroenT30: 2102369-block-title-30.patch queued for re-testing.
Comment #34
vijaycs85This is good to go.
Comment #35
catchWhy add this on the controller but not the interface? It's only used internal to the class so maybe that's OK and we say this an internal implementation detail of the base class, but I don't think this was discussed. It could also be protected if it's not in the interface.
Comment #36
catchComment #37
tim.plunkettI don't think it was explicitly discussed, but I agree with this statement. It's fine as is.
Comment #38
catchWorks for me. Committed/pushed to 8.x, thanks!