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
HTML characters aren't escaped/encoded in the media embed caption field. Anything with the > character causes everything before > to get truncated, breaking the embed.
Which is to say, this:
produces this:
Proposed resolution
I THINK from rooting around that the entity embed popover caption field is supposed to encode any HTML characters for decoding on render.
(...and if someone can figure this out, allowing HTML in the caption, would it be possible to wrap that caption field in the popover with a ckeditor instance of its own, so that editors can have inline buttons for link/strong/em/ul/etc so they don't have to write raw html?)
Remaining tasks
User interface changes
Possibly a ckeditor instance in the popup?
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2688427-double-escape-captions.patch | 1.92 KB | Dave Reid |
| |||
| screengrab 2016-03-16 at 10.27.10 AM.png | 92.62 KB | itmaybejj | |
| screengrab 2016-03-16 at 10.26.58 AM.png | 90.89 KB | itmaybejj | |
Comments
Comment #2
itmaybejj CreditAttribution: itmaybejj at Princeton University commentedComment #3
itmaybejj CreditAttribution: itmaybejj at Princeton University commentedMy apologies for talking to myself...
As I poke around at this, I'm realizing the data-caption approach might work fine if the html characters could be properly encoded...so this might just be an encoding/escaping problem.
Strong/em/a are very common in captions...I could imagine list items too...but in practice -- if we could encode html, couldn't we slap a ckeditor profile on this and give it a set of buttons in the modal? Then users could turn rich text on and off, and pick their own buttons...
Comment #4
itmaybejj CreditAttribution: itmaybejj at Princeton University commentedrewriting summary as a bug report; feature request was based on my thinking the approach was the problem; on further rooting around I think the approach is fine but there is a bug on the modal dialog...
Comment #5
itmaybejj CreditAttribution: itmaybejj at Princeton University commentedComment #6
balsamaThanks John. Migrating this to Entity Embed after speaking with Dave Reid.
Comment #7
Dave Reid@itmaybejj: What input format were you using while embedding the content?
I definitely agree that editing the caption inline in the editor would be preferred.
Comment #8
balsama@Dave Reid - I can't speak for @itmaybejj, but since this was originally filed against Lightning, he was probably using the text format that comes with that. The config for that format can be found here:
https://github.com/acquia/lightning/blob/8.x-1.x/modules/lightning_featu...
Comment #9
Dave Reid@balsama: Thanks, I'm definitely able to replicate with that configured text format + CKEditor (thanks D8 for making it easy to import arbitrary config!)
Comment #10
Dave ReidBased on #2105841: Xss filter() mangles image captions and title/alt/data attributes it looks like we need to ensure that captions are double-escaped, not just escaped as an attribute.
Comment #11
Dave ReidHere's the patch version of the pull request at https://github.com/drupal-media/entity_embed/pull/212 that should resolve it, for anyone that needs this in a distro or project.
Comment #12
Dave ReidNote that any existing captions likely would need to be re-edited to be encoded properly in the filtered text fields.
Comment #13
Dave ReidWe already have a postponed issue to make the caption editable in the WYSIWYG: #2282957: Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog)
Comment #15
Dave ReidCommitted the fix to 8.x-1.x!
Comment #16
itmaybejj CreditAttribution: itmaybejj at Princeton University commentedThis is my happy dance. Thank you all!
Comment #18
Wim LeersA follow-up for this issue was created: #2907616: Follow-up for #2688427: hook_entity_embed_values_alter() does not pass $form_state.