Updated: Comment 0

Problem/Motivation

Now that _title can be added to the route definition we realized that it does not work in connection with forms.
The problem is that it does not use the same code path.

Proposed resolution

For now add the _title to the separate codepath as well.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#13 title-FAIL.patch1.31 KBdawehner
#13 title.patch6.3 KBdawehner
#12 title-2068437-12.patch4.8 KBdawehner
#10 title-2068437-10.patch2.79 KBdawehner
#8 form_title-2068437-8.patch3.07 KBpwolanin
#8 2068437-5-8.increment.txt1.69 KBpwolanin
#5 form_title-2068437-PASS.patch2.33 KBdawehner
#5 form_title-2068437-FAIL.patch1.58 KBdawehner
#1 routing-2068437-1.patch765 bytesdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new routing-2068437-1.patch765 bytes

.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +WSCCI

.

aspilicious’s picture

aspilicious commented
Status: Needs review » Reviewed & tested by the community

Fixes the problems I had

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

No tests...

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new form_title-2068437-FAIL.patch1.58 KB
new form_title-2068437-PASS.patch2.33 KB

I had added the tag :p

aspilicious’s picture

aspilicious commented

:s srry webchick. I talked about those with dawehner but totally forgot.

Dawehner it's the route one that will fail not the one build with the render array.

dawehner’s picture

dawehner
Primary language German
commented

Oh, this is maybe just broken on old menu router items? Does your route implement hook_menu and page callback?

pwolanin’s picture

pwolanin commented
StatusFileSize
new 2068437-5-8.increment.txt1.69 KB
new form_title-2068437-8.patch3.07 KB

per #2032535: Resolve 'title' using the route and render array we shoudl also have _title_callback. Also, the code was in the wrong section of the conditional.

however, parallel work is hapenning on the vire controller yu Crell, so postponing any test addition

dawehner’s picture

dawehner
Primary language German
commented

Sorry but the title callback is out of scope of this issue, let's do one step after the other.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new title-2068437-10.patch2.79 KB

.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +blocker

Looks great and blocks converting things to remove drupal_set_title() from core (and contribs). Eg. I ran into this with #2070055: drupal_set_title() is deprecated. Agreed we don't need to support title callback as core does not do it either.

dawehner’s picture

dawehner
Primary language German
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new title-2068437-12.patch4.8 KB

The problem is that the title issue was not tested with overlay enabled. I think we have to add an explicit test for that.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new title.patch6.3 KB
new title-FAIL.patch1.31 KB

Adding a test for overlay to show the failure.

yesct’s picture

yesct commented
Issue tags: -Needs tests

I think it has tests now. Any others?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Thanks for covering overlay. Looks good for me :)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Automatically closed -- issue fixed for 2 weeks with no activity.