Comments

vijaycs85 created an issue. See original summary.

vijaycs85’s picture

Issue summary: View changes
matthiasm11’s picture

CKEditor 4.8.0 has been released on the 13th of december, can we please get this in D8?
https://github.com/ckeditor/ckeditor-releases/releases/tag/4.8.0

matthiasm11’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new3.62 MB

Downloaded a custom build (with the Drupal build-config.js) and all languages on CKEditor.
Created a patch against te latest 8.5.x-dev.

The patch also applies to 8.4.3.

saravanaprasanth’s picture

Assigned: Unassigned » saravanaprasanth
saravanaprasanth’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new24.75 KB

Hi matthiasm11,

I have applied the above CKEditor library for the 8.4.3 version. The patch got applied successfully and The CKEditor library updated to the version 4.8.0.

Later tested the CKEditor it was working as expected.

Thank You.

saravanaprasanth’s picture

Assigned: saravanaprasanth » Unassigned
shawn dearmond’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Needs review

Shouldn't this be applied to 8.4.x-dev? Not 8.5.x-dev?

This fixes a pretty critical bug: #2911749: [upstream] CKEditor 4.7 regression: Styles dropdown broken

cristian100’s picture

#4 works for me on D 8.4.3

vijaycs85’s picture

Status: Needs review » Reviewed & tested by the community

Looks like patch applies on both 8.4.x and 8.5.x. Back to RTBC.

wim leers’s picture

Version: 8.4.x-dev » 8.5.x-dev
Issue tags: +Needs manual testing

This needs manual testing. Looks like #8 already did some of that, I'd like to do some more.

In principle, we don't commit new CKEditor minors to Drupal 8 minors. But in this case, it's fixing a significant bug, and so therefore core committers might chose to still do so. Risk should be very limited (but that's also what we thought when we updated Drupal 8.4 to CKEditor 4.7… that's the first time CKEditor let us down in a major way).

Stay tuned for more feedback.

wim leers’s picture

Version: 8.5.x-dev » 8.4.x-dev

Oops, meant to not change the version.

larowlan’s picture

Version: 8.4.x-dev » 8.5.x-dev

8.4.x will only receive critical backports now

simgui8’s picture

#4 works for me on 2 different 8.4.4 website

thank you

wim leers’s picture

Issue tags: -Needs manual testing
  1. Manual testing didn't reveal any problems.
  2. In HEAD, core/assets/vendor/ckeditor/ckeditor.js is 564K, with the patch applied, it's 576K. Not much we can do about that.
  3. I also built CKEditor 4.8.0 with our build-config.js, and it does match. (Just with "2018" instead of "2017", slightly different JS minification and different ordering of translation strings.

Therefore, confirming RTBC.

wim leers’s picture

Updated IS to match previous "Update CKEditor library" issues, and reference those.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, manually put this through its paces: tested toolbar configuration, formatting, tables, images + captions, etc. Couldn't find any problems, and confirmed the only change outside of core/vendor is to core.libraries.yml.

So let's get this in since, as @larowlan pointed out in Slack, "more eyes make shallow bugs." :)

Committed and pushed to 8.5.x. Thanks!

  • webchick committed c433063 on 8.5.x
    Issue #2926932 by matthiasm11, Wim Leers, vijaycs85: Update CKEditor...
wim leers’s picture

Excellent!

Status: Fixed » Closed (fixed)

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

oriol_e9g’s picture

Nobody has detected problems with Firefox 58 on Windows with this patch? Works perfectly with Chrome but with Firefox 58+Windows disappears some icon buttons. All buttons works but some of them has not picture.

Maybe it's a problem with my instalations, not with this patch.

oriol_e9g’s picture

It's a problem with core\assets\vendor\ckeditor\skins\moono-lisa\icons.png file. I have downloaded the file from CKEditor and now it's working opn Firefox, maybe it's a problem with my git or file system config.

matthiasm11’s picture

@oriol_e9g, just tested the wysiwyg on Windows 10 + Firefox 58, all icons are rendering fine on my project.

wiifm’s picture

Can anyone re-roll a patch that applies against 8.4.7 to get CKEditor version "4.8.0+2018-04-18-security-patch" ? Due to $REASONS we cannot yet upgrade to Drupal 8.5.2.

jribeiro’s picture

New patch that applies against 8.4.7.

wiifm’s picture

Patch applies perfectly and does what it says on the box, thanks so much @jribeiro!

wim leers’s picture

#24: See https://www.drupal.org/sa-core-2018-003, the 8.4.7 release already includes a patched CKEditor. That's right: both the 8.4.x and 8.5.x branches got security releases!

jribeiro’s picture

in my case, I'm using the #25 patch to fix #2911749: [upstream] CKEditor 4.7 regression: Styles dropdown broken, I'm not sure if the patched version on 8.4.x is fixing that. I'll check.

wiifm’s picture

Hey @Wim Leers, Drupal 8.4.7 only includes CKEditor 4.7.2 + sec patch. We also need to upgrade CKEditor to 4.8.0 to avoid #2911749: [upstream] CKEditor 4.7 regression: Styles dropdown broken. Drupal 8.5.2 includes this version, but do to factors beyond our control, we cannot yet upgrade to that version of core just yet. Thus, the patch @jribeiro supplied is still needed.