The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
One of the comments on #2863904: Typogrify does not handle langcode properly mentions a PHP warning:
Warning: Illegal string offset ',,' in Drupal\typogrify\Plugin\Filter\TypogrifyFilter->tips() (line 457 of modules/contrib/typogrify/src/Plugin/Filter/TypogrifyFilter.php).
Proposed resolution
The error comes from the code
foreach (UnicodeConversion::map('quotes') as $ascii => $unicode) {
if ($settings['quotes'][$ascii]) {
// ...
}
}
The quick fix is to change the condition to something like !empty($settings['quotes'][$ascii]).
I think it would be better to figure out why $settings['quotes'][',,'] is not set. It should be set in the settingsForm() method. That is, when the settings form is submitted, that nested array value should be set.
Remaining tasks
User interface changes
Fewer PHP messages.
If any of the quote types are configured, then they will be mentioned on the Text formats help page (/filter/tips).
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3024682-8.patch | 1.16 KB | benjifisher |
| #5 | 3024642-5.patch | 899 bytes | rithesh bk |
| #2 | typogrify-3024642-2.patch | 870 bytes | jigish.addweb |
Comments
Comment #2
jigish.addweb commented@benjifisher, Please find my patch that works for your mentioned suggestion, it adds the !empty condition on TypogrifyFilter.php file.
Thanks!..
Comment #3
benjifisher@jigish.addweb:
I did not really suggest that. I called it a "quick fix". I would like to know why
$settings['quotes'][',,']is not set.Still, having this patch on the issue is a good thing. It means that people using the module can apply it, and it should get rid of the PHP warnings. Thanks for the contribution!
I see on your profile that you mentor new contributors. Perhaps you can help one of them to fix #3024641: Undefined variable notice when viewing filter help text. For that issue, the quick fix is the correct fix.
Comment #4
rithesh bk commentedi am working on it .....
Comment #5
rithesh bk commentedPlease find the updated patch ......
Comment #6
rithesh bk commented@jigish.addweb #2 patch is not applying ..... So i created new patch file . Please find the updated working patch file #5 with the changes suggested by benjifisher in #3
Comment #7
benjifisher@Rithesh BK:
Thanks for updating the patch: the one in #2 no longer applies after #3024641: Undefined variable notice when viewing filter help text.
Your patch is still a quick fix. That is, I am afraid that it fixes the symptom of the problem without actually fixing the problem. If we want to replace
',,'with'„', will it work?I am setting the status back to NW. Let's understand the problem before proposing a solution.
Comment #8
benjifisherAs I feared, there was an underlying problem that was not fixed by the added checks in the earlier patches.
Pay attention to the warning message: it is "Illegal string offset", not the more familiar "Undefined index" (which is a Notice, not a Warning). I searched for that message, which led me to Illegal string offset Warning PHP on StackOverflow. (Ignore the accepted answer. The others are better informed.)
The problem is that, in the expression
$settings['quotes'][$ascii](see the issue summary)$settings['quotes']is a string, not an array. (And the inner index$asciiis also a string. It is legal to index strings by integers, but not by strings.)A little debugging shows that
$settings['quotes']is the serialized empty array,'a:0:{}'.The solution is to unserialize
$settings['quotes']before using it.Once we do this, we get the more familiar notice, "Undefined index". Now it makes sense to apply the extra check from Comment #2.
Comment #9
benjifisherIt is not good practice to review one's own work, but atr least the testbot agrees with me that this patch looks OK. I want to get this issue fixed, so I am going to review it myself: RTBC.
Comment #11
benjifisherComment #12
benjifisher