Problem/Motivation
Before: the contact category title you set was used as the page title for the contact form.
Currently: not anymore.
This now fails some of our fine tests in https://drupal.org/project/config_translation, so we cannot move forward with building out that module and ensure tests pass. Therefore marked blocker.
Proposed resolution
Make the label appear as the title again.
Comment | File | Size | Author |
---|---|---|---|
#20 | contact-title-fix-20-tests-only.patch | 1.66 KB | Gábor Hojtsy |
#20 | contact-title-fix-20.patch | 3.08 KB | Gábor Hojtsy |
#18 | contact-title-fix-18-tests-only.patch | 1.67 KB | Gábor Hojtsy |
#18 | contact-title-fix-18.patch | 3.08 KB | Gábor Hojtsy |
#11 | contact-title-fix-10-tests-only.patch | 1.67 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyHere are quick untested fixes. The problem is #2089635: Convert non-test non-form page callbacks to routes and controllers and #2032535: Resolve 'title' using the route and render array in combination broke this because now the title is provided in the route, so the title callback is not considered, and the fixed title is used at all times. We should use #title in the page callbacks. Also made that change for the personal contact form.
Comment #2
Gábor HojtsyComment #3
andypostLet's incorporate the change in #1938390: Convert contact_site_page and contact_person_page to a new-style Controller
Comment #4
Gábor HojtsyComment #5
Gábor HojtsyFeel free to incorporate this change there, but this fails config_translation tests and therefore blocks any chance that could land in core, so I prefer not to depend on a patch going on since March. :/ Sorry. Trying to be agile here.
Comment #7
andypostcheck_plain()?
PASS_THROUGH was lost
Comment #8
Gábor HojtsyRight, how do you do passthrough with #title?! I'd have done it if it would be evident to me :D
Comment #9
andypost$form['#title'] = String::checkPlain($category->label());
//not $entity
Comment #11
Gábor HojtsyWith checkplain :)
Comment #12
Gábor HojtsyComment #14
Gábor Hojtsy#11: contact-title-fix-10-tests-only.patch queued for re-testing.
Comment #15
Gábor Hojtsy#11: contact-title-fix-10.patch queued for re-testing.
Comment #17
tim.plunkettYou still need this for the breadcrumb, for now.
Comment #18
Gábor HojtsyUpdated!
- Removed the .module hunk as pointed out by @tim.plunkett this is needed for the breadcrumb (see #17)
- The personal page test was checking the wrong username, we should check the admin user who's contact form is loaded.
- Changed the assert in the personal test to raw since we escape the username with @ already
This passes fine for me :) Any other concerns? :)
Comment #19
andypostAdded the changes to #1938390-108: Convert contact_site_page and contact_person_page to a new-style Controller
please do not use deprecated functions - should be $this->admin_user->getUsername()
Comment #20
Gábor HojtsyIt is fascinating how much we can refine 10 lines of code. Amazing.
I of course used user_format_name() because the original code used that too so I just copy-pasted that out of the drupal_set_title() (which was not even broken, so just wanted to double up to fix that in this regression while I was at it). Updated that in code and test.
Comment #21
andypostNow it's amazing ;)
Comment #22
catchCommitted/pushed to 8.x, thanks!
Comment #23
Gábor HojtsyYay, thanks!