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
"Convert line breaks into HTML" filter is placing the `drupal-media` tag within a paragraph. It should exclude this tag, as it excludes others, such as `script` and `iframe`.
This is an alternative solution to #3097338: If a filter wraps the drupal-media tag in a paragraph, the edit button breaks, where the currently recommended fix is to fix drupalmedia plugin.js to be more flexible about what it encounters inside the widget wrapper.
This was only discovered now because we failed to test media embedding with the "Convert line breaks into HTML" filter enabled. Now more people are using Drupal 8.8 and are trying out the media embed feature, so they are reporting some configurations we didn't test against when building the feature. The "Convert line breaks into HTML" filter isn't really necessary with CKEditor, but the checkbox to enable it doesn't have that fact clearly displayed.
Proposed resolution
Add the `drupal-media` tag to the list of tags to exclude.
Remaining tasks
review, discuss, commit
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3100066-2.patch | 1.74 KB | oknate |
| #2 | 3100066-2--FAIL.patch | 897 bytes | oknate |
Comments
Comment #2
oknateComment #3
oknateComment #5
alison#2 applies cleanly and is effective at fixing the behavior for me, thanks! -- core 8.8.0
Comment #6
oknateThanks for testing. To clarify, @alisonjo315, you mean it fixed #3097338: If a filter wraps the drupal-media tag in a paragraph, the edit button breaks, is that correct?
Comment #7
alisonMmmm I mean that, with this patch (#3100066 > #2), the rendered "embedded media" aren't getting wrapped in
<p>anymore. (Does that clarify, or?)Comment #8
oknateYes, that clarifies. Thanks!
Comment #9
Wim LeersSimple test coverage that shows the problem.
Simple solution for the problem.
There's nothing more to ask for :) Great job!
Comment #10
Wim LeersThe issue summary should explain why this is only discovered now (AFAIK: because most Drupal 8 sites use CKEditor, and hence don't use
filter_autopanymore).The issue summary should also be updated to explain that this is an alternative solution to #3097338: If a filter wraps the drupal-media tag in a paragraph, the edit button breaks.
Comment #11
oknateUpdated the issue summary based on Wim's feedback in #10. Also, we may want to backport this to 8.8, as users are running into the bug described in #3097338: If a filter wraps the drupal-media tag in a paragraph, the edit button breaks that is caused by this.
Comment #12
Wim LeersBugfixes are always backported to the current minor :) They just need to be committed to the next minor first. So don't worry about that! 😊
Comment #13
acbramley CreditAttribution: acbramley at PreviousNext commentedI was having an issue where the Edit Media button wasn't appearing and in my browser console I was getting
Cannot read property 'insertBeforeMe' of null plugin.js:242@phenaproxima pointed me to here and I confirmed that disabling the filter_autop filter fixed the error and caused the button to start appearing (yay!). It looks like this patch will solve that issue too :)Comment #14
Wim LeersFix title nit.
Comment #15
phenaproximaThis bug is quite a nuisance, and possibly a straight-up disruption, in Drupal 8.8 and later. It even breaks media embedding in Umami (which uses the autop filter). The fix is non-disruptive, and the value of fixing it greatly (IMHO) outweighs any open questions about where or how this should be dealt with.
So, I strongly believe this should be backported to 8.8.x.
Comment #16
phenaproximaOpened a follow-up at @lauriii's request to discuss making this configurable: #3102154: Allow filter_autop to be configured to ignore certain tags.
Comment #19
lauriiiCommitted 2c4ace9 and pushed to 9.0.x and 8.9.x. Thanks!
Leaving open to consider backporting this to 8.8.x
Comment #20
oknateThank you! +1 for Adding to 8.8.x branch! We're getting questions from users about this in the media slack channel, where it causes the 'Edit Media' button to fail, and it's not immediately obvious why.
Comment #21
oknateUnassigning from myself.
Comment #22
alisonWith this patch alone (i.e. without the patch from #3097338), my edit button is broken again, with the JS error we discussed over on #3097338. Just mentioning in case it helps other folks.
(I don't have filter_autop enabled anyway, but for some reason, the fix on #3097338 is effective for my situation -- sadly, I don't have steps to reproduce, etc., and I can't troubleshoot further right now -- really just sharing in case it's helpful for anyone else!)
Comment #23
Wim Leers@alisonjo315 Which filters do you have enabled? Can you share the source markup so that we can reproduce it?
Comment #24
alexpott+1 for the backport.
Comment #25
alexpott@larowlan also thinks this is good for backport so I've cherry-picked this to 8.8.x