Status messages styling broken with big pipe after 10.3 drupal core update#428
Status messages styling broken with big pipe after 10.3 drupal core update#428ioanaa20 wants to merge 2 commits intoopeneuropa:1.xfrom
Conversation
ioanaa20
changed the title
| * The full URL for the BCL icons. | ||
| */ | ||
| function _oe_bootstrap_theme_get_icon_path(): string { | ||
| $path = &drupal_static(__FUNCTION__); |
There was a problem hiding this comment.
Nice find here +1, this is called hundreads of times per page load, all to return the same result everytime, so statically caching this makes sense.
This does create some noise in the PR and could also belong to a dedicated issue if maintainers prefer that.
| // Check if the BCL icon path is set in the current theme. | ||
| if (!empty($theme_info['openeuropa']['bootstrap_component_library']['icon_path'])) { | ||
| $bcl_icon_path = $theme_info['openeuropa']['bootstrap_component_library']['icon_path']; | ||
| $bcl_icon_path = Html::escape($theme_info['openeuropa']['bootstrap_component_library']['icon_path']); |
There was a problem hiding this comment.
I'm not sure we need this actually.
This value is coming from code (theme .info.yml file so not really user input).
In twig this is already escaped on output, bcl-icon.html.twig:33
In JS I think it is better to use document.createElement + element.settAttribute, instead of element.innerHtml and a string literal as it's done right now (messageWrapper.innerHTML += svgIconHtml;).
|
|
||
| return messageWrapper; | ||
| }; | ||
| })(Drupal, drupalSettings); |
There was a problem hiding this comment.
I did not check the JS implementation in detail but I think we should consider copy/pasting the one from ui_suite_bootstrap instead which seems to cover a lot more things.
https://git.drupalcode.org/project/ui_suite_bootstrap/-/blob/5.2.x/assets/js/misc/message.js?ref_type=heads
Radix uses the same (copy pasted from ui_suite_bootstrap): https://git.drupalcode.org/project/radix/-/blob/6.0.x/js/message.js?ref_type=heads
2 participants
Jira issue(s):
Worked on the issue created here:
Status messages styling broken with big pipe after 10.3 drupal core update #419