Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wiifm’s picture

Sorry about the whitespace fixes in the patch, my editor trims them automatically ;)

mkesicki’s picture

@wiifm ,
thank you for your patch. We try to check it as soon as possible. Please be patient.

wiifm’s picture

Any idea on how likely this is to be accepted? I would prefer to be able to roll with a standard (unpatched) install of ckeditor ;)

wiifm’s picture

Uploading a new version of the patch that is --relative (for drush make)

jddh’s picture

Issue summary: View changes
FileSize
949 bytes

I'm running into the same issue with version 7.x-1.16. For this version, I needed to patch the dialog JS instead—it was stripping out any number values that included px or &.

Status: Needs review » Needs work

The last submitted patch, 5: mediaembed.patch, failed testing.

rakenodiax’s picture

FileSize
1.02 KB

Reroll #5 for latest 7.x-1.x-dev

vokiel’s picture

Status: Needs work » Needs review
FileSize
1.03 KB

The idea from #7 seems to be working just fine, although I think is too wide as \S* matches any non-whitespace character (equal to [^\r\n\t\f\v ]).

For the width/height we can have digits or digits with the unit, eg width="300" or width="0.50vw" or eventually width="50%".

So there is a need just for: [a-zA-Z0-9\.\%] or even a whitelist of all possible units. That would require more updates though, so I would just stick to easier but not too wide regex.

See attached proposal.

  • vokiel committed 7d15d66 on 7.x-1.x
    Issue #1635788 by wiifm, jddh, rakenodiax, vokiel: Media embed is...
vokiel’s picture

Status: Needs review » Fixed
ron_s’s picture

While I understand the purpose of this patch from 9 years ago, I don't believe it has much value today.

Embedded media in modern websites should not have hard-coded width and height settings, otherwise it is impossible to properly scale on mobile devices. The much better approach is to standardized aspect ratios that will display correctly on any device size.

This can easily be done as a preprocess for any media such that it is seamless to content contributors.

Status: Fixed » Closed (fixed)

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