From #1981644: Figure out how to deal with 'title/title callback'. Updated: Comment #121
Commits
Patch in #78 was committed in #81.
Problem/Motivation
As seen on #1974408: Convert aggregator_page_source() to a Controller, routes defined via the route system does not have yet the concept of titles. Titles
are not only needed for menu links, but also for the actual title on the page, so things which has been MENU_CALLBACK's before also have titles.
Proposed resolution
Title for a page will be determined by a hierarchy of sources.
Menu links and breadcrumb building will first look for _title_callback; if that's found, it will use the return STRING from that callable. If not found, it will use the string in _title. If that's not found, there is no title. If _title_callback returns a string, it is assumed to be already translated.
HtmlPageController will support the same logic (callback wins over static), but will continue to also support the #title property on the content array as a final override. That way there's not an additional API break with what we just put in. :-) However, the use of #title will be discouraged unless you want the title to be different between the page and the link. This parallels calling drupal_set_title() from within a page callback in D7, which was always inferior to using "title callback".
The solution consists of several points:
- Route definitions can contain a _title property which will be used for static titles
route_name:
pattern: '/route-pattern'
defaults:
_content: ...
_title: 'Static title' - Route definitions can contain a _title_callback property which will be used for dynamic titles The value of _title_callback will be a callable definition (identical syntax to _content and _controller). _title_callback will be called using reflection in the same way as _content and _controller, so it has access to all the same parameters.
route_name:
pattern: '/route-pattern'
defaults:
_content: ...
_title_callback: 'MyClass::titleMethodName' - If you need an even more dynamic title or override you can return it as part of the main render array. This is like drupal_set_title() in 7 and is discouraged:
class controllerClass { class controllerMethod() { return array( '#title' => 'Dynamic title' . mt_rand(0, 10), ); } }
API changes
Related Issues
#1974408: Convert aggregator_page_source() to a Controller
#1889790: [Meta] Allow modules to register links for menus, breadcrumbs and tabs (if not with hook_menu)
#1830588: [META] remove drupal_set_title() and drupal_get_title()
#1839338: [meta] Remove drupal_set_*() drupal_add_*() in favour of #attached/response for out-of-band stuff
#2023795: REGRESSION: hook_local_actions doesn't use title callback
#2027183: hook_menu() title callback is ignored on routes
#2031473: Convert menu local actions to plugins so that we can generate dynamic titles and paths
#2076085: Resolve the need for a 'title callback' using the route (follow-up)
#2076059: Missing documentation for 'title/title callback' using the route and render array (follow-up)
#2068437: _title does not work on _form (follow-up to make forms work)
Comment | File | Size | Author |
---|---|---|---|
#109 | title-callback-2032535-109.patch | 1.3 KB | pwolanin |
#108 | title-callback-2032535-108.patch | 1.22 KB | pwolanin |
#101 | title-2032535-101.patch | 657 bytes | dawehner |
#78 | 2032535-title.patch | 18.85 KB | Crell |
#75 | title-2032535-74.patch | 18.85 KB | dawehner |
Comments
Comment #1
dawehnerMaybe just this is already enough?
Comment #2
Crell CreditAttribution: Crell commentedRenaming for uniqueness. Probably could do with a better name than this.
Comment #3
dawehnerThe only way how to get information from the subrequest to the mainrequest seem to be the header information, so this patch introduces a new "drupal_page_title" header which stores the title.
This title can be either specified in the route information or directly in the render array.
Comment #5
katbailey CreditAttribution: katbailey commentedDiscussed this in person with effulgentsia and msonnabaum and briefly on irc with dawehner, larowlan and beejeebus. My feeling is that the subrequest handling that happens with these non-response-returning controllers is causing more problems than it solves (in fact, it's not clear to me that it solves any problem at all..?)
So I'm trying an approach that changes the HtmlPageController to not result in a subrequest being handled at all and instead just call the controller directly and wrap the result in drupal_render_page().
I've merged this with the other changes in #3 that did not affect HtmlPageController. Sorry for the lack of an interdiff, it was a bit messy - I originally started out with the patch in #1830588-30: [META] remove drupal_set_title() and drupal_get_title().
Comment #7
katbailey CreditAttribution: katbailey commentedComment #8
dawehnerI really really like the approach.
I guess this should be a full sentence.
Let's use the same code path for both the if and elseif.
+1 for making the html page controller way easier to understand!
Just a general question: Now that we don't need subrequests anymore, would it make sense to add a title method on the controller? This would bring controllers neared to blocks.
Comment #9
fubhy CreditAttribution: fubhy commentedYes, I would very much like that. I had a discussion with @sdboyer and @EclipseGc (and also daniel for some time of the call) on exactly that topic. A title method would be very cool.
Comment #10
Crell CreditAttribution: Crell commentedSeems like this should be a more self-documenting constant. Even with context it's hard to grok.
I'd much rather keep HtmlPageController not-container aware and just inject the controller resolver. This isn't a factory, so it shouldn't be able to get away with being container aware. We just removed container aware from it recently. :-)
Comment #11
dawehnerAs better name for the ControllerInterface was suggested, though this would have blown up the patch size, so I went with a TitleControllerInterface.
Comment #13
dawehnerForgot the Title class.
Comment #15
dawehnerThis time with even less missing files (TitleControllerInterface)
Comment #16
pwolanin CreditAttribution: pwolanin commentedI don't understand - I don't think this:
can ever work since $callable is an array
also: core/includes/theme.inc: patch does not apply
Comment #17
pwolanin CreditAttribution: pwolanin commentedFixes conflict in theme.inc, and I think fixes the logic of the $callable.
Comment #18
damiankloip CreditAttribution: damiankloip commentedSounds like that needs to be tested if the changes you just made are a fix.
Comment #19
pwolanin CreditAttribution: pwolanin commented@damiankloip - yes - unclear why the test code didn't fail before. Also, I think dawehner or someone else needs to double-check the changes to theme.inc
Comment #20
dawehnerassertTitle seems to check just the < title > element, but not the h1 element on the page.
Comment #21
Crell CreditAttribution: Crell commentedTagging
Comment #22
dawehnerDoes anyone has a problem with removing the TitleInterface and just use the render array?
Comment #23
Crell CreditAttribution: Crell commentedTitleInterface--
Comment #24
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis smells like a gross hack. At the minimum, let's set in stone a
[obj, method_name]
return value for ourControllerResolver::createController
.Comment #25
Crell CreditAttribution: Crell commentedDamien: Impossible. createController() returns a PHP callable. A PHP callable could be 5 different things: function name, closure object, object implementing __invoke(), class name and static method name, or object and method name. Only two of those will be returned as an array.
That said, if TitleInterface goes away then I don't think we need that code anyway.
Comment #26
Damien Tournoud CreditAttribution: Damien Tournoud commented@Crell: we can decide in our own implementation to restrict the callable to be a array of object and method name. That's perfectly compatible with behavioral subtyping. It's very very far from being impossible by any definition of "impossible".
Comment #27
dawehnerThis time without the title interface.
Comment #29
dawehnerNo wonder that this does not work.
Comment #30
Crell CreditAttribution: Crell commentedMy read here is that _title overrides a returned #title. I'd think it should be the other way around: Runtime wins over hard coded.
This is just a test class. We don't need it translated. No t() please. (It's not worth injecting, either.)
In fact the test for it would fail if the text was translated to something else. -:-)
Comment #31
dawehnerYeah I agree.
Comment #32
Damien Tournoud CreditAttribution: Damien Tournoud commentedBy calling
createController()
directly, we support only theservice:method
andclass:method
notations and we are missing the other cases handled directly bygetController()
(aka. function names, object-method pairs, lambdas, classes). Are we happy with that?Should be
!isset()
.Comment #33
dawehnerWell, at least our createController implementation always returns array($object, $method), but it is right that we at somehow internal dependencies.
Let's check for the empty title.
Comment #34
Crell CreditAttribution: Crell commentedWhy checking for a #title of ''? If the controller explicitly specified no title, we should have no title. That's a valid thing to do.
Comment #35
katbailey CreditAttribution: katbailey commentedPer discussion in irc, we'll do some refactoring so that we'll have a method for getting the callable from a controller, which will take care of all possibilities (addressing DamZ's first point in #32).
Comment #36
dawehnerAs talked about in IRC, this adds a new method, which allows you to cover all cases of getController.
Comment #38
dawehnerForgot the new interface
Comment #39
dawehner#38: title-2032535-38.patch queued for re-testing.
Comment #41
dawehnerRerolled.
Comment #42
Crell CreditAttribution: Crell commentedThere's no "attribute" here. The whole point of this method is that there's no concept of an attribute; just a string in one of the various formats we support.
I don't think we want this in the default. If a controller returns a string, my assumption is that it should have whatever default title was specified, not force no title.
Pre previous comment, if an empty title was explicitly set, we should honor that. "Use whatever the default is" is indicated by returning no #title at all.
Comment #43
dawehnerWe (tstoeckler and yesct) talked about that, and we came to the conclusion that we don't have a better idea. If we could rename getController we would name it getControllerFromRequest so getController would be make sense for the new method we introduce here. For sure we can't do that, so a "YesCT: possible solution for the name is getControllerFromNotRequest or getControllerFromDefaultAttribute?". Other suggestions have been getControllerFromRoutingSchema or getControllerFromRoutingDefinition or getControllerFrom ... Do you maybe have an idea?
Good review!
Let's fix it, see interdiff.txt
Comment #44
msonnabaum CreditAttribution: msonnabaum commentedPerhaps out of scope, but I think the reason getControllerFromAttribute is an awkward name, is because it's on the wrong object.
That method should be on a new Method Object, something like CallableParser::parse($callable_string).
Comment #45
Crell CreditAttribution: Crell commentedStrictly speaking we're not getting a controller. We're getting a callable. getCallable() sounds good to me.
(In IRC, Mark pointed out that getCallable() shouldn't be part of ControllerResolver but its own object. Maybe, but that's out of scope here so let's not worry about that.)
Comment #46
aspilicious CreditAttribution: aspilicious commentedRequest isn't used, does it needs a use statement?
Comment #47
dawehnerGood catcfh aspilicous!
If we don't get a RTBC in the meantime I will maybe just do it at somepoint here.
Comment #48
dawehnerSo, this time with a patch.
Comment #49
Crell CreditAttribution: Crell commentedWe need at least a better method name before we can RTBC. Given how much this blocks I'm OK with not factoring it out to a separate service just yet; that's a non-breaking follow-up IMO.
Comment #50
dawehnerWhat about getControllerFromDefinition, as we are working with route definitions (not sure though whether this is the proper word).
Comment #51
fubhy CreditAttribution: fubhy commentedI think we should call it PSR-3 in the documentation. Also, I've never seen "(optional)" in this context.
Unrelated. But, whatever...
I'd agree that we need a separate service thing for this. Potentially even just a new Drupal\Components component for resolving arguments for callables. Our ControllerResolver could then simply reference and re-use that one.
Can be a follow-up though.
"to load 'a' controller".
Also, "some errors made by the developer" sounds weird. :P (we don't make mistakes anyways, eh?). Not sure about this entire sentence tbh.
Hm? What's that for? :)
I don't find it particularly appealing to stuff yet another attribute into the request but I can't think of an equally simple solution for D8 so I guess this is fine. Putting it into the route definition is not very nice easier but it's the best we can do for now I guess. At least this is much better than what we have now (nothing). So let's just do it for now and further improve it for D9.
"related 'to' drupal page title'. (I believe)
Also completely unrelated.
Comment #52
dawehnerThanks for your review!
Oh right this is looking way better. Well, the logger might not be there, see constructor.
let's remove it then.
Do you want to create a follow up for it?
This is all coming from symfony, so let's remove it.
Okay, let's also remove it.
Not really, because the file got renamed.
Comment #53
Crell CreditAttribution: Crell commentedThis line is German grammar, not English. :-)
"If no title was returned fall back to one defined in the route."
Otherwise I think we're good here. It would be nice to factor the callable creation logic out to a stand-alone class, but if we really want that can be done later in a BC-compatible way.
Comment #54
dawehnerAre you sure this is even german?
Comment #56
Crell CreditAttribution: Crell commented#54: title-2032535-54.patch queued for re-testing.
Comment #57
Crell CreditAttribution: Crell commentedNo, but grammatical errors involving the word "allows" seem to mostly come from German developers. I have no idea why. :-)
(And bah, bot!)
Comment #58
jibranExtra white space and it still needs issue summary update.
Comment #60
andypostEDIT sorry the testbot was broken
#54: title-2032535-54.patch queued for re-testing.
Comment #62
dawehnerIf someone would have said that there is a problem with the twig iniative in a routing issue I would have laughed.
Comment #63
xjmI believe this is critical for its impact on #1830588: [META] remove drupal_set_title() and drupal_get_title(), which I've postponed on this, even if the actual menu system change is not in itself release-blocking (which it might or might not be).
Let's get the API changes for this issue documented in the issue summary so that a core maintainer can sign off on them.
Comment #65
dawehner#62: title-2032535-62.patch queued for re-testing.
Comment #67
dawehner#62: title-2032535-62.patch queued for re-testing.
Comment #68
catchTagging - we need to change the API so we can rip out drupal_set_title(), I'm a week or so out of date on this issue so didn't review most recent approach.
Comment #69
dawehnerAdapted the title.
Comment #70
tim.plunkettWhy add the interface and then not typehint by it?
I guess this needs to be added to the places l.d.o checks for strings
Comment #71
dawehnerRerolled the patch.
Good catch!
Right, is there some kind of meta or another process to add them?
Comment #72
Crell CreditAttribution: Crell commentedI'm satisfied, and this blocks other issues. Let's call it done.
Comment #73
Crell CreditAttribution: Crell commented#71: title-2032535-71.patch queued for re-testing.
Comment #75
dawehnerThere we go, a straight one-file rerole.
Comment #77
donquixote CreditAttribution: donquixote commentedTo clarify:
- This does sufficiently and elegantly replace drupal_set_title().
- It does replace the "title" value from D7 hook_menu().
- It does not replace the "title callback" from D7 hook_menu(). Unless we want to build the entire render array to get the dynamic title value. Which we usually want to avoid when we are not on that page already. E.g. to build a breadcrumb from entity urls with aliases.
This being said, I still agree with this patch as a step forward.
Whether we do something about "title callback" or let it die, can be decided in a follow-up, if at all (I seem the only one interested in it).
And apologies if I missed something on the way. I said most I had to say in the beginning (of the other issue). Then I deliberately took some distance from this subject, to avoid the temptation of repeating myself all over..
All I am asking now is to be clear about this in the issue summary and such.
Comment #78
Crell CreditAttribution: Crell commentedI have no idea why the block.routing.yml hunk was not applying, but let's try this.
Comment #78.0
Crell CreditAttribution: Crell commentedUpdated issue summary.
Comment #79
Dries CreditAttribution: Dries commentedI'm still new to this patch but is there a reason why adding a _title property doesn't result in $variables['page']['#title'] being set? It seems a bit odd to to add all these if-tests in the preprocess functions instead of putting some logic in a centralized place like the HtmlPageController.
Comment #80
Crell CreditAttribution: Crell commentedCurrently that function serves both the old pipeline and the new pipeline, so we can't guarantee that page isn't coming from the old routing system. That means we have to account for both cases.
I'm working on a cleanup of that now that helps to smooth out the whole new-style pipeline that will likely clean that up further (including centralizing that logic in the HtmlPageController). Either way, that can get tidied up when we remove the old router.
Comment #81
Dries CreditAttribution: Dries commentedAlright, that makes sense. Looking forward to the clean-up patch. Committed to 8.x. Thanks. Marking 'needs work' because we'll want to create a change notification.
Comment #82
Crell CreditAttribution: Crell commentedRetitling and filing accordingly.
Comment #83
dawehnerUpdated the existing one for the routing system: https://drupal.org/node/1800686/revisions/view/2806137/2807105
Added a new change notice on https://drupal.org/node/2067859
Comment #84
Crell CreditAttribution: Crell commentedLooks good to me. Thanks!
Comment #85
Gábor HojtsyWell, so this now requires the translation extractor to also look at routing.yml files. Is there any plan to support the menu description as well in this, or is that going to be / is already at a totally different place? I mean items in the admin backend where we used to use hook_menu() only for the title and description at this point because the route handled the association of path to controller and access checking. I've opened #2069923: Support for Drupal 8 routing.yml title to support this in the extraction system. Awaiting clarity on description so we can ensure the translation system supports all cases.
(Aside: if we put the title and description into the route, then how is this at all different from hook_menu() with a different syntax?)
Comment #86
aspilicious CreditAttribution: aspilicious commentedGabor we don't need hook_menu anymore. If we leave title in hook_menu we also need to implement hook_menu.
I do miss the custom title callback option.
Comment #87
Gábor Hojtsy@aspilicious: right, my question was what happens to this item:
Ok, title goes to the routing file or the render array. What happens to description? There is a lack of mention of that on this issue and the change notice. If the title goes to the route, then everything (title, "callback", access) is now on the route for this item, except the description. Is there an issue for that too that I missed and was not mentioned above? I feel like I missed something and need to ensure this is covered for translations, so kind of need the info :)
Comment #88
Crell CreditAttribution: Crell commentedThe description is relevant only to the LINK. The title in the YAML file is for the PAGE. hook_menu() conflated those two. The split routing/menu-links systems do not (at least not automatically). That said, we can probably tweak menu links from hook_menu to inherit their title from the route if one is not specified.
Comment #89
Gábor Hojtsy@Crell: ok, in short, what's the *code* to make that description appear without hook_menu? (It may only be me seeing a connection between title and description, but both need to have stable support in translations so this needs info).
Comment #90
dawehnerAs the description belong to the menu link it will be either on hook_menu or hook_menu_link or however it will be called.
Comment #91
webchickI'm with Gábor on this. I don't care how they're represented under the hood, but "title" and "description" are a unit as far as far as UI text is concerned. It makes absolutely no sense to define one in one place and the other somewhere else.
Comment #92
Gábor HojtsyI don't have strong opinions on this. The routing, tabs and title are already defined at different places. All I was looking for is information on what happens to description. Looks like we don't know yet and since I did not find an issue yet, that seems like still to be opened. We need hook_menu() for now then for that at least.
Comment #93
Crell CreditAttribution: Crell commentedGabor: I think you're conflating ROUTER ITEMS with MENU LINKS, which hook_menu in D7 and earlier did. There is no code to make description appear without hook_menu(), because hook_menu() is where it makes sense. We needed title to be available sans-menu link entity. We don't need description available sans-menu link entity.
So nothing has changed with regards to description translation.
Comment #94
Gábor Hojtsy@Crell: I don't think I'm confusing anything with anything. People said above that hook_menu() is not needed anymore, eg. #86, and where description goes was never explained, I needed explanation on that so I can open / work on the right issues to ensure support for them is not broken. Whatever the answer to that question I just asked for answers :)
That said, tabs and local actions also work as menu links, and they are annotated classes now. So I think it was logical to assume description has a plan some way. Seems to be the only thing left for hook_menu that does not have a solution elsewhere.
Comment #95
tim.plunkettI think #86 was an over-simplification. Until this went in, we either needed hook_menu for *routing* for the page title, or we had to add a drupal_set_title to the controller. Now a route is truly standalone from hook_menu().
Menu links, and their titles (not the page title) are still dependent on hook_menu().
Comment #96
Gábor HojtsyIn other words, although the issue title says "Resolve 'title/title callback' using the route and render array" it does not have anything to do with hook_menu() at all (or title callback). hook_menu() titles and descriptions are still there and supported and should be used. The code committed here does not "resolve" that from the router item then I guess(?).
Comment #97
Crell CreditAttribution: Crell commentedThe thing "resolved" is that there was no way to set a title for a page that wasn't in the menu via hook_menu. Now there is. hook_menu() is at this point really just hook_menu_link_entities(). :-) See also: #2047633: Move definition of menu links to hook_menu_link_defaults(), decouple key name from path, and make 'parent' explicit
Comment #98
xjmThis patch did not add any documentation for the new
_title
route thing nor#title
. The change notice is great, but we also need these things actually documented in the codebase.I couldn't sort out where to document either. It looks like both are handled by HtmlPageController, so I couldn't decide whether it makes sense to add
#title
to thedrupal_render()
docs or not. Also, does#title
do anything when it's deeper than the full page array?Similarly, where are the
_routingwhatsit
thingies documented?Reopening rather than creating a followup since this is a serious documentation gate issue.
Comment #99
xjmAlso, we need to mark
drupal_set_title()
@deprecated
and add an@see
to wherever the new docs end up.Comment #100
xjmRelated: #2046367: Document the internals of the router
Comment #101
dawehnerI don't think that there is any place in Drupal yet, where the new routing system is documented.
Here is something which deprecates just drupal_set_title().
Comment #102
xjmThanks @dawehner!
For now let's add:
drupal_render()
docs, with an explanation that it is specific to the page array, not a part of Render "API", and an @see to HtmlPageController.template_preprocess_page()
as well about where the title is coming from.We can then do more with it once #2046367: Document the internals of the router is in.
Couple other thoughts:
Also hmm,
drupal_get_title()
callsdrupal_set_title()
. We need to deprecate that too and add it to the change notice. Also seems like we need a followup to remove it there.I'm wondering if it would make sense to use this class for more than just a bucket of constants? We could add methods and docs to it as appropriate if that were the case.
Comment #103
star-szrCNW based on #102.
Comment #104
pwolanin CreditAttribution: pwolanin commented?
Comment #105
donquixote CreditAttribution: donquixote commentedIf we don't use _title for menu or breadcrumb or any other links, then what is the benefit over #title in the page render array?
And should we not name it _page_title then instead?
One benefit I could see is that one could alter the _title from yml in some way..
Comment #106
pwolanin CreditAttribution: pwolanin commentedI still think we need to optionally have a title callback - I don't understand why that was removed from the patch.
Comment #107
Crell CreditAttribution: Crell commentedWe hashed through this in today's IRC meeting. Conclusion:
Routes will have an additional option parameter added, _title_callback
The value of _title_callback will be a callable definition (identical syntax to _content and _controller). _title_callback will be called using reflection in the same way as _content and _controller, so it has access to all the same parameters.
Menu links and breadcrumb building will first look for _title_callback; if that's found, it will use the return STRING from that callable. If not found, it will use the string in _title. If that's not found, there is no title. If _title_callback returns a string, it is assumed to be already translated.
HtmlPageController will support the same logic (callback wins over static), but will continue to also support the #title property on the content array as a final override. That way there's not an additional API break with what we just put in. :-) However, the use of #title will be discouraged unless you want the title to be different between the page and the link. This parallels calling drupal_set_title() from within a page callback in D7, which was always inferior to using "title callback".
Someone please update the issue summary with that. :-)
Comment #107.0
Crell CreditAttribution: Crell commentedupdate issue summary
Comment #107.1
pwolanin CreditAttribution: pwolanin commentedUpdated issue summary.
Comment #107.2
pwolanin CreditAttribution: pwolanin commentedUpdated issue summary.
Comment #108
pwolanin CreditAttribution: pwolanin commentedHere's an initial patch - need to add an implementation and tests.
Comment #109
pwolanin CreditAttribution: pwolanin commentedoops used has() instead of get() and the wrong method on the controller resolver
Comment #110
Crell CreditAttribution: Crell commentedThis is going to conflict with the HtmlPage issue, but it's small enough that I should be able to reroll it. I'm half tempted to just RTBC and be done with it but I'll leave it open for a bit to get other weigh-in.
We may want to try and convert at least one example while we're here just to prove that it works. :-)
Comment #111
tim.plunkettNeeds an example and a test.
Comment #112
webchickThis may be a stupid question, but could we not just have some magic so that if _title is a string, it becomes a title, but if _title is a callable, it uses it as a callback to generate the title? Fewer properties to memorize++, and the word "callback" in the name is confusing since that traditionally means "a global function" and what you mean here is "any callable, but usually a method on a class," no?
Comment #113
tim.plunkett@webchick, what if the name of your route was 'debug'? That's a callable :)
_title_callable and _title_callback were both tossed around today, callable is the PHP typehint but callback matches the hook_menu closer.
I'm not sure what to call it, but I don't think we should try to tack it onto _title.
Comment #114
Gábor Hojtsy@webchick: please, no! If the _title may be a callable or may be user facing text, how would the translation extractor know to extract it for translation or not. Hint: it would have no idea.
Comment #115
dawehnerIf it is a callable in the sense of either a method on an object or a service there would be at least a colon, but yeah this means we can't go with it.
Comment #116
webchickOk, stupid idea then. :P Carry on. :P
Comment #117
xjm#108 is a new thing and should go in its own issue; @pwolanin will file a separate issue for that. Meanwhile I filed #2076059: Missing documentation for 'title/title callback' using the route and render array so that can move forward separately. Docs are a gate and should not be postponed on things that don't exist yet.
Comment #118
pwolanin CreditAttribution: pwolanin commentedfollow-up for _title_callback : #2076085: Resolve the need for a 'title callback' using the route
Comment #118.0
pwolanin CreditAttribution: pwolanin commentedUpdated issue summary.
Comment #119
xjmRetitling to clarify how this is different from the other one so I stop confusing them in my tracker. :)
Comment #120
Gábor HojtsyFor me, even $page['#title'] does not seem to work in its simplest form: #2070055: drupal_set_title() is deprecated
Comment #121
YesCT CreditAttribution: YesCT commentedI was looking at the patch in #78 that was committed in #81 to see if it had a test that was using #title to override a title, as the example in the issue summary example #3 in #2076085: Resolve the need for a 'title callback' using the route . But the test seems to set a title, not override one. @dawehner pointed out forms are different. And that #2068437: _title does not work on _form is needed and has a test with a form.
#2070055: drupal_set_title() is deprecated (in config_translation) is marked postponed on 2068437.
I'll add it to the issue summary here as a follow-up to this issue.
Comment #122
pwolanin CreditAttribution: pwolanin commented@Gábor Hojtsy - can you post a snippet of what you are trying?
Comment #122.0
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #122.1
YesCT CreditAttribution: YesCT commentedadded followup to make forms work, and issue to add docs. added commit patch and comment number.
Comment #123
thedavidmeister CreditAttribution: thedavidmeister commentedWhy was this added to template_preprocess_maintenance_page()?
$variables['page'] can't exist there at all AFAICS. hook_theme() only allows 'content' for maintenance pages.
system_region_list($GLOBALS['theme'])
for core provides the following regions:Stark:
- sidebar_first
- sidebar_second
- content
- header
- footer
- highlighted
- help
- page_top
- page_bottom
Seven:
- content
- help
- page_top
- page_bottom
- sidebar_first
Bartik:
- header
- help
- page_top
- page_bottom
- highlighted
- featured
- content
- sidebar_first
- sidebar_second
- triptych_first
- triptych_middle
- triptych_last
- footer_firstcolumn
- footer_secondcolumn
- footer_thirdcolumn
- footer_fourthcolumn
- footer
Comment #124
dawehnerAdded a follow up with the question: #2088957: title in template_preprocess_maintaince_page should be on 'content' not 'page'
Comment #125
star-szrComment #126.0
(not verified) CreditAttribution: commentedtrying to note which are follow-ups and not just "related"