Coming from a core issue #3352384: Add Exception for TypeError Argument must be String in Drupal Component Utility Html escape{} which is not clear enough yet regarding core handling of such issues and by helping a user with a TypeError issue and to find its reasons, I made this finding. Many reports come up on D.O. lately because of the change to strict in PHP handling of Type. From the users report to me:
"After updating to latest module version and a successful user login, a WSOD appears with the following TypeError message on screen.
TypeError: htmlspecialchars(): Argument #1 ($string) must be of type string, array given in htmlspecialchars() (line 432 of core\lib\Drupal\Component\Utility\Html.php)Making the admin path unmaintainable."
This is not the only issue like this, but I was sure each is caused by another reason. So I started to make some points and brought up some thoughts on this in Slack and had a very helpful chat with @smustgrave who suggested to print the array content in the moment the error gets initiated. So I did the following for the users case: Changing the respective code in the spot of the error message from
public static function escape($text): string {
if (is_null($text)) {
@trigger_error('Passing NULL to ' . __METHOD__ . ' is deprecated in drupal:9.5.0 and will trigger a PHP error from drupal:11.0.0. Pass a string instead. See https://www.drupal.org/node/3318826', E_USER_DEPRECATED);
return '';
}
return htmlspecialchars($text, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
}
to
public static function escape($text): string {
if (is_array($text)) {
var_dump($text);
}
if (is_null($text)) {
@trigger_error('Passing NULL to ' . __METHOD__ . ' is deprecated in drupal:9.5.0 and will trigger a PHP error from drupal:11.0.0. Pass a string instead. See https://www.drupal.org/node/3318826', E_USER_DEPRECATED);
return '';
}
return htmlspecialchars($text, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
}
to print a var_dump from an array which actually should be string here. Which leads to the following output in place of the admin menu section.
web/core/lib/Drupal/Component/Utility/Html.php:431:
array (size=2)
'flag' =>
array (size=3)
'#theme' => string 'flags' (length=5)
'#code' => string 'de' (length=2)
'#source' => string 'language' (length=8)
'title' =>
array (size=1)
'#markup' => string 'DE' (length=2)
web/core/lib/Drupal/Component/Utility/Html.php:431:
array (size=2)
'flag' =>
array (size=3)
'#theme' => string 'flags' (length=5)
'#code' => string 'de' (length=2)
'#source' => string 'language' (length=8)
'title' =>
array (size=1)
'#markup' => string 'DE' (length=2)
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | Screenshot 2024-09-27 at 13.16.23.png | 8.88 KB | james.williams |
Issue fork toolbar_language_switcher-3378536
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:
Comments
Comment #2
dqdComment #3
dqdComment #4
dqdComment #5
dqdComment #6
dqdComment #7
chris64@diqidoq, could you provide an associated stack trace?
Comment #8
james.williamsThis module currently sets the 'title' properties in the render array to a string / TranslatableMarkup, whereas the posted stack trace shows it has been set to an array (containing a #markup), alongside a 'flag' render array (using the 'flags' theme hook).
But there are no mentions of 'flag' or 'flags' in the toolbar_language_switcher codebase (2.0.x-dev at least); are you running any other modules that could be adding these? Perhaps this module is currently incompatible with one you're using, because it assumes the language names would be strings rather than render arrays? (Which is why turning this module off would fix your problem of course.) If we can establish which module is conflicting, perhaps we can consider building support for language switcher links that are render arrays rather than simple strings. I suppose
\Drupal\Core\Language\LanguageManagerInterface::getLanguageSwitchLinks()never actually promises what will be inside the array of links.Adjusting priority to normal, since this will only affect sites using the combination of conflicting modules, which is not possible with core and the most common contrib modules. (At least until we can be shown otherwise!)
Comment #9
chris64In #7 stack trace means the functions list. With a WSOD such a list may be in the log.
Comment #10
volegerSetting the appropriate status of the issue.
Comment #11
yurii novak commentedThe error occurs when using the "languageicons" module.
Comment #12
james.williamsAh great! Thanks for the specific use case. That patch would solve the WSOD, but presumably replaces whatever the languageicons module wanted to do? Perhaps we can catch render arrays and allow them to be rendered, instead? But I wonder what that would look like, given what languageicons wants to set it to. Render arrays could quite easily produce markup that is simply far too big to fit inside the toolbar item.
Changing to a feature request, as this is really about integrating with other modules that change the type of input data to something unexpected.
Comment #13
dqd#8 & #12 Yes and no. It does not hijack the issue but it somewhat turns the issue around a little bit. But if it helps to see w/t/hat needs to be done, then I leave it up to what ever we call it.
Agreed with FR. And as you can see in the related issue link, I actually changed the issue summary of the previously discussed core issue I originally came from in a similar way back in the days... But just let me add this question: Which default would break less? The chance is high that menu links turn back and forth between render arrays and strings for so many reasons that I think it does not need to be listed here. I am even not sure if the new Drupal 11 core navigation module renders its link titles as render arrays. Oh and not to mention Superfish, link badges, and what not sure to be come. But never mind, as I sad in the summary, I actually came from a core issue regarding the problem of raising TypeError issues all over the places, possibly most of them based on PHP getting more strict on this or Drupal more vague, depending on individual setups. And that we maybe need more conditional handling of this in many code bases. Or Exceptions. This is what I have suggested in core. Especially in cases when the type can change. Which seem to the issue on many spots. And this is exactly what my original issue summary brings up in a way, considered that I am not a native tongue. And well, actually your title change covers it too. But maybe it should be changed one more time into something like:
"Support language switcher links that can vary in type of string or render array" or in case you do not want an extending FR "Provide exception if switcher links turn into render arrays"
so that both scenarios are covered.
Comment #14
dqdNo idea what happened with the last code block in the issue summary back in the days (looked like a mess) ... changed it accordingly for readability and left out the last assumption sentence since the scope if the issue maybe changes a bit . In case I have time to come back on this I probably would start with tests of different typical multi language Drupal setups to see if and how often the link title type changes then.
Comment #15
james.williamsThanks for the additional detail, and yes, I think you've put that very well. Let's ensure the module doesn't run into any kind of error. Whether that means just showing the language name as in comment 11 (as a bug fix), or supporting render arrays (as a feature request), I accept that either would be an improvement. And it doesn't really matter what kind of fix this is; I'd just like it to be a good one :)
Perhaps we could support render arrays and see how those from the languageicons module look. Maybe even with something like a max-height / overflow: visible rule to avoid seriously breaking the toolbar appearance if any other modules try to render something especially large?
If none of us get time to work on supporting render arrays any time soon, I'll probably accept the patch from comment 11 that just replaces arrays with the language name.
Comment #17
james.williamsOK, I've opened MR !6 to handle render arrays. I've tested with and without the languageicons module; I believe it works quite nicely! I do love seeing all the little flags in my toolbar now :-)
Please take a look and review/test if you can; thanks!
Comment #18
james.williamsHere's a screenshot showing how the switcher links look with the languageicons module:
Comment #20
volegerI'm going forward, as changes look good for me.