Problem/Motivation

9.2.x and 9.3.x and 9.1.x contain CKEditor 4.17.0 code but should be 4.16.1

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#3 3215929.patch1.51 MBlarowlan

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Title: Fix CKEditor version in libraries.yml » Fix CKEditor versions
Issue summary: View changes
larowlan’s picture

Status: Active » Needs review
StatusFileSize
new1.51 MB

Status: Needs review » Needs work

The last submitted patch, 3: 3215929.patch, failed testing. View results

spokje’s picture

This is all a bit messy (I know, SA-pressure...), but as far as I can tell:

- This issue is a follow-up from: #3215916: Update ckeditor to 4.16.1, where CKEditor was bumped to 4.17
- That happened for 9.2.x and 9.3.x, but not for 9.1.x.
The latter being the reason the patch won't apply to 9.1.x, since CKEditor 4.16.1 is already/still in there after SA-CORE-2021-003.
- The test-fail on 9.3.x seemed like a random one, ordered retest. As soon as that comes back green, I'm going to RTBC

spokje’s picture

Assigned: Unassigned » spokje
Issue summary: View changes

Removed 9.1.x from IS

spokje’s picture

Assigned: spokje » Unassigned
Issue summary: View changes

I stand corrected 9.1.9 was shipped with CKEditor 4.17 (See https://git.drupalcode.org/project/drupal/-/blob/9.1.9/core/assets/vendo...), so 9.1.x should be patched.

By now, I'm confused about why we're going backwards in CKEditor versions, leaving dev-branches using older stuff than actual releases, so I'll re-add 9.1.x to the IS and leave this issue alone.

spokje’s picture

At least it seemed I was right about something, the 9.3.x fail has been previously logged as random: #3203712: [random test failure] EntityDisplayTest::testExtraFields()

effulgentsia’s picture

I'm confused about why we're going backwards in CKEditor versions

The SA intended to update to 4.16.1, but unintentionally pulled in additional commits ahead of that tag. 4.17.0 has not yet been released, so going to 4.16.1 is in order to get onto a tagged release.

spokje’s picture

Status: Needs work » Reviewed & tested by the community

Thanks @effulgentsia for clearing that up. :)

I've build the Drupal version of CKEditor 4.16.1 locally according to the instructions in core/assets/vendor/ckeditor/build-config.js and got the same result as 3215929.patch, except for the expected cache-buster query string.

So patch 3215929.patch is RTBC for me on 9.3.x and 9.2.x.

To prevent 9.1.x holding up those branches, I've opened follow-up #3215970: [9.1.x] Fix CKEditor versions for that branch

spokje’s picture

  • catch committed a5bb99c on 9.3.x
    Issue #3215929 by larowlan, Spokje, effulgentsia: Fix CKEditor versions
    

  • catch committed fe0ebf6 on 9.2.x
    Issue #3215929 by larowlan, Spokje, effulgentsia: Fix CKEditor versions...
catch’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

spokje’s picture

Issue tags: +Bug Smash Initiative

Status: Fixed » Closed (fixed)

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