Problem/Motivation
#2663316: Broken title in modal dialog when title is a render array introduced a regression for routes that return a string or array response.
Steps to reproduce
https://www.drupal.org/project/moderation_sidebar is one module that causes this error:
Error: Call to a member function render() on string in Drupal\Core\Render\MainContent\DialogRenderer->getTitleAsStringable()
Proposed resolution
Just like the #title check, support both that and getTitle() responses to be an array.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3443959
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3443959-dialogrenderergettitleasstringable-does-not
changes, plain diff MR !7806
Comments
Comment #3
berdirPushed a fix, not sure about tests here. This is a pretty severe regression from 10.2, completely breaks a module like moderation_sidebar and the new implementation is quite a bit simpler (and also a bit faster, since it avoids calling getTitle() twice.
Comment #4
berdirLooking at the changes again, the previous implementation explicitly supported #title as NULL, I can't really imagine how that would be desirable and there don't seem to be any tests for that? But it would be easier to expand the ?? to an if/else again if we want to keep that.
Comment #5
catchI think we should have test coverage for the bug, but also I think we might want to open a dedicated issue for that and fix the regression before 10.3.0 beta so that various contrib aren't stuck with an unresolvable blocker. Bumping to critical and tagging as beta target.
The other option would be rolling back the original issue, but I didn't review the original change there to see what that would look like, except that it needed a CR so might not be easy to get back into 10.3.x again.
Comment #6
smustgrave commentedOpened #3445469: Add additional test coverage for DialogRenderer::getTitleAsStringable() for the test coverage to not hold up 10.3.
Comment #11
catchCommitted/pushed to 11.x and cherry-picked to 11.0.x, 10.4.x, 10.3.x, thanks!