Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Prior to #3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) there were scenarios where:
GHS attributes are not retained in linked media when no additional GHS attributes are allowed on
<drupal-media>element.This would allow retaining
data-barin linked media:
<drupal-media data-foo> <a data-bar>However, this does not, even though it should:
<a data-bar>
We should add a test to confirm this is in fact addressed.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3276217
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:
- 3276217-drupalmedia-ghs-attributes
changes, plain diff MR !2335
1 hidden branch
Comments
Comment #2
Wim LeersThere are already two issues related to
<a href="…"><drupal-media … /></a>markup:Are you sure this is a completely independent third?
Comment #3
lauriiiYes, this is unrelated to those. However, I believe this is entirely on our side. This is related to how we are integrating to GHS in
DrupalMediaGeneralHtmlSupport.Comment #4
Wim Leers👍
Comment #6
lauriiiI don't think there's a way to reproduce this now that #3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) has been committed because that ensures that when
<drupal-media>is enabled, there's always additional attributes added to it via GHS. I added test coverage for this, but on top of that, we may want to harden the code a bit even though there isn't any way to test that now.Comment #8
Wim Leers… only if
filter_htmlis enabled though, right? So for "Full HTML" that won't be the case.But your test is proving that it works fine for the "Full HTML" use case too. So … what hardening are you thinking of?
(Also reviewed the MR: basically only nits.)
Comment #9
lauriiiI'm not sure this ever was broken for the Full HTML case – I have only tested this in the restricted use case. I also realized that the Full HTML use case wouldn't be solved by that issue, hence the test coverage to ensure that Full HTML case also works as expected.
If you look at https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/cked..., it's clear that the GHS processing for the link attributes is only added when there is also GHS attributes added for
<drupal-media>element. That is wrong, but there isn't any realistic use cases where that would cause issues now that #3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) is in. 🤷♂️Comment #10
Wim LeersWell then … I don't see what further hardening we would need. Especially now that we have test coverage for this. 👍😄
Will RTBC as soon as the nits are addressed 🥳
Comment #11
lauriiiComment #12
Wim Leers🚢
Comment #14
bnjmnmComment #15
bnjmnmComment #20
bnjmnmTest works nicely. Committed to 10.0.x and cherry picked to 9.5.x, 9.4.x, and 9.3.x