Comments

thpoul created an issue. See original summary.

thpoul’s picture

Issue summary: View changes
thpoul’s picture

Status: Active » Needs review
StatusFileSize
new1.36 MB

Status: Needs review » Needs work

The last submitted patch, 3: 2724225-3.patch, failed testing.

thpoul’s picture

Status: Needs work » Needs review
wim leers’s picture

Issue summary: View changes
Priority: Minor » Normal
Status: Needs review » Reviewed & tested by the community
Related issues: +#2698587: Update CKEditor library to 4.5.8

Thanks! This looks great. Manually tested, works just like before.

This should also be committed to Drupal 8.1.x because it fixes #2702171: [upstream] CKEditor's Moono skin does not respect the CKEditor cache-busting query string when loading its icons sprite, which means that any site currently still on Drupal 8.0 that upgrades to 8.1 won't experience that WTF anymore.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new1.36 MB

I've rebuilt ckeditor using build.sh and the patch I get is not the same. Steps I took to build ckeditor...

  1. git clone https://github.com/ckeditor/ckeditor-dev
  2. git co 4.5.9
  3. copied build-config.js from Drupal's vendor/assets/ckeditor to dev/builder
  4. Ran sh build.sh -s from dev/builder
  5. Copied ckeditor folder over Drupal's vendor/assets/ckeditor

Given this is minified JS we need to be careful about what we add - how come this generates a different patch from #3?

thpoul’s picture

@alexpott: the only difference I see between the 2 patches is the timestamp: G4JC vs G4CG

alexpott’s picture

@thpoul - yep I think you're right and it doesn't look like there is any of setting the timestamp during the build process.

wim leers’s picture

Assigned: Unassigned » mlewand

I've pinged the CKEditor team to gather feedback.

mlewand’s picture

You guys are absolutely correct, the timestamp is unique and is different for every build.

wim leers’s picture

Status: Needs review » Reviewed & tested by the community

Alright, thanks for confirming :)

wim leers’s picture

Assigned: mlewand » Unassigned
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 25628e3 and pushed to 8.1.x and 8.2.x. Thanks!

Committed to the 8.1.x since this was a ckeditor patch release for us too and it fixes #2702171: [upstream] CKEditor's Moono skin does not respect the CKEditor cache-busting query string when loading its icons sprite

  • alexpott committed 2a6e399 on 8.2.x
    Issue #2724225 by thpoul, alexpott, Wim Leers, mlewand: Update CKEditor...

  • alexpott committed 25628e3 on 8.1.x
    Issue #2724225 by thpoul, alexpott, Wim Leers, mlewand: Update CKEditor...
wim leers’s picture

Yay! Closed #2702171: [upstream] CKEditor's Moono skin does not respect the CKEditor cache-busting query string when loading its icons sprite because this fixed that, and unpostponed #2239419: Include CKEditor's AutoGrow plug-in. This will allow us to further improve the CKEditor experience in Drupal :)

Status: Fixed » Closed (fixed)

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