Update to CKEditor 4.5.9. http://ckeditor.com/release/CKEditor-4.5.9

4.5.9 brings minor bugfixes plus it unblocks #2239419: Include CKEditor's AutoGrow plug-in

See also:

#1858210: [meta] Content editing experience follow-ups — in-place editing and WYSIWYG
#1950098: Update CKEditor library to 4.1
#2036253: Update CKEditor library to 4.2
#2039163: Update CKEditor library to 4.4
#2271051: Update CKEditor library to 4.4.4
#2345961: Update CKEditor library to 4.4.5
#2384581: Security: Update CKEditor library to 4.4.6
#2415111: Update CKEditor library to 4.4.7
#2521820: Update CKEditor library to 4.5.3
#2321583: Update CKEditor library to 4.5.5
#2663566: Update CKEditor library to 4.5.7
#2698587: Update CKEditor library to 4.5.8

CommentFileSizeAuthor
#9 2724225-9.patch1.36 MBalexpott
#3 2724225-3.patch1.36 MBthpoul
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thpoul created an issue. See original summary.

thpoul’s picture

thpoul CreditAttribution: thpoul at Pixual commented
Issue summary: View changes
thpoul’s picture

thpoul CreditAttribution: thpoul at Pixual commented
Status: Active » Needs review
FileSize
2724225-3.patch1.36 MB

Status: Needs review » Needs work

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

thpoul’s picture

thpoul CreditAttribution: thpoul at Pixual commented
Status: Needs work » Needs review
mlewand’s picture

mlewand CreditAttribution: mlewand commented

It also solves #2702171: [upstream] CKEditor's Moono skin does not respect the CKEditor cache-busting query string when loading its icons sprite. :)

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
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.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Related issues: +#2702171: [upstream] CKEditor's Moono skin does not respect the CKEditor cache-busting query string when loading its icons sprite
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs review
FileSize
2724225-9.patch1.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

thpoul CreditAttribution: thpoul at Pixual commented

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

@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

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: Unassigned » mlewand

I've pinged the CKEditor team to gather feedback.

mlewand’s picture

mlewand CreditAttribution: mlewand commented

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Alright, thanks for confirming :)

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: mlewand » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
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

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

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.