Problem/Motivation

Media embed attributes are rendered in the container div when preview is rendered in editing view. This includes the alt attribute, which is not a valid attribute on div element.

Proposed resolution

Do not render attributes on the preview container div in editing view.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 3248440-4-d10.patch57.5 KBlauriii
#4 3248440-4.patch57.64 KBlauriii
#4 3248440-4-test-only.patch1.02 KBlauriii

Comments

lauriii created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: wim leers at Acquia commented
Title: Media embed attributes are rendered in container div in editing view » [drupalMedia] Media embed attributes are rendered in container div in editing view
Project: CKEditor 5 » Drupal core
Version: 1.0.x-dev » 9.3.x-dev
Component: Code » ckeditor5.module
Issue tags: +Needs tests, +JavaScript

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Active » Needs review
Issue tags: -Needs tests
StatusFileSize
new 3248440-4-test-only.patch1.02 KB
new 3248440-4.patch57.64 KB
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented
Status: Needs review » Reviewed & tested by the community

This looks straightforward to me :)

The last submitted patch, 4: 3248440-4-test-only.patch, failed testing. View results

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

This doesn't apply to 10.x - can we get a separate patch for that? - thanks.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
StatusFileSize
new 3248440-4-d10.patch57.5 KB

Here's a D10 version of #4.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: wim leers at Acquia commented

This looks straightforward to me :)

+1, very nice!

  • bnjmnm committed 68a3e85 on 10.0.x 
    Issue #3248440 by lauriii, Wim Leers, nod_: [drupalMedia] Media embed...

  • bnjmnm committed e73d2d1 on 9.4.x 
    Issue #3248440 by lauriii, Wim Leers, nod_: [drupalMedia] Media embed...

  • bnjmnm committed 68b338b on 9.3.x 
    Issue #3248440 by lauriii, Wim Leers, nod_: [drupalMedia] Media embed...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Reviewed & tested by the community » Fixed

Agree it is a straightforward change, the attributes shouldn't be assigned to the container on editingDowncast. Added to 10.0.x, 9.4.x, as well as 9.3.x as it is experimental.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.