Problem/Motivation

  1. https://github.com/ckeditor/ckeditor5/releases/tag/v36.0.0
  2. https://github.com/ckeditor/ckeditor5/releases/tag/v36.0.1

Steps to reproduce

Proposed resolution

  1. Update core/package.json
  2. cd core
  3. yarn install
  4. yarn build
  5. yarn build:ckeditor5-types

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CKEditor has been updated to 36.0.1

CommentFileSizeAuthor
#13 3344083-13-9.5.x.patch3.83 MBlongwave
#7 interdiff.txt1005 byteslauriii
#7 3344083-7.patch3.83 MBlauriii
#6 Screenshot 2023-03-10 at 12.24.33.png46.04 KBricardofaria
#6 Screenshot 2023-03-10 at 12.21.46.png40.72 KBricardofaria
#5 update-ck-editor-3344083-4.patch995.15 KBricardofaria
#2 3344083-2.patch3.83 MBlongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3344083-2.patch3.83 MB

Status: Needs review » Needs work

The last submitted patch, 2: 3344083-2.patch, failed testing. View results

ricardofaria’s picture

ricardofaria commented

Filed for me too. Adding a new patch.

ricardofaria’s picture

ricardofaria commented
StatusFileSize
new update-ck-editor-3344083-4.patch995.15 KB
ricardofaria’s picture

ricardofaria commented
StatusFileSize
new Screenshot 2023-03-10 at 12.21.46.png40.72 KB
new Screenshot 2023-03-10 at 12.24.33.png46.04 KB

I've made new tests locally.
Both patches on #2 and #5 work.

On #5 it is for CK editor 36.0.1.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new 3344083-7.patch3.83 MB
new interdiff.txt1005 bytes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Update CKEditor 5 to 36.0.0 » Update CKEditor 5 to 36.0.1
Issue summary: View changes
Issue tags: +JavaScript, +10.1.0 release notes
Parent issue: » #3326896: Update CKEditor 5 to 35.4.0
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Issue tags: -JavaScript +JavaScript
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
+++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php
@@ -1599,7 +1599,10 @@ public function testViewMode(bool $with_alignment) {
+    $this->assertNotEmpty($dropdown = $this->getBalloonButton('View Mode 1'));
...
     $this->assertNotEmpty($this->getBalloonButton('View Mode 1'));

This looks to fix the test fail from #2, but isn't this just asserting the same thing twice now?

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

@longwave I'm reading it as

Is it visible
Is it not empty, guess this could be valid as maybe its visible but empty?

Certainly seems like additional coverage but not sure it's 100% the same thing. Could very well be wrong.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

I added the assertNotEmpty before clicking just to have better error messages in case the button doesn't exist. We could potentially remove the second assertion but I don't see any harm asserting that the button exists after the dropdown tray has been opened. It also makes it clear that we expect three buttons to exist there.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 3344083-13-9.5.x.patch3.83 MB

Patch for 9.5.x.

  • catch committed 9a527be3 on 10.1.x 
    Issue #3344083 by longwave, lauriii, Wim Leers, smustgrave: Update...

  • catch committed 2c79d953 on 10.0.x 
    Issue #3344083 by longwave, lauriii, Wim Leers, smustgrave: Update...

  • catch committed 8567ae25 on 9.5.x 
    Issue #3344083 by longwave, ricardofaria, lauriii, Wim Leers, catch:...
catch’s picture

catch
he/him
Primary language English
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: -10.1.0 release notes +9.5.6 release notes, +10.0.6 release notes

Committed/pushed to 10.1.x, cherry-picked to 10.0.x, and committed the 9.5.x patch to 9.5.x, thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Normal » Critical
Status: Fixed » Needs work
Related issues: +#3350252: LinkIt requires new minor version for Drupal 10.1.x, since CKEditor 5 got a major version update (v35 → v36)

Woah, I did not know this was going to land in Drupal 9.5 & 10.0 too! That's why in #8 I specifically tagged it 10.1.0 release notes… 😅

I thought our policy was to keep every Drupal core minor on the same CKEditor 5 major, and only update to CKEditor 5 minor/patch releases (which indeed are very rare)?

CKEditor 5 major releases introduce BC breaks. And this is not theoretical, e.g. contrib modules are breaking due to this: #3350252: LinkIt requires new minor version for Drupal 10.1.x, since CKEditor 5 got a major version update (v35 → v36). Contrib modules providing custom CKEditor 5 plugins will often (but not always) have to create a new minor release specifically for the next core minor to chase this. It will often be as simple as re-building using yarn, but it's still necessary.

IMHO we should revert this commit in 9.5.x and 10.0.x and unpublish the 9.5.6 and 10.0.6 releases, and redo them without this one commit. 😬🙈

  • catch committed 8706ac5d on 10.0.x 
    Revert "Issue #3344083 by longwave, lauriii, Wim Leers, smustgrave:...
bradjones1’s picture

bradjones1
Primary language English
commented

Pretty sure releases can't be unpublished but this may be justification for a narrowly-focused minor version release?

  • catch committed 61951c7c on 9.5.x 
    Revert "Issue #3344083 by longwave, ricardofaria, lauriii, Wim Leers,...
catch’s picture

catch
he/him
Primary language English
commented

I've reverted this. Probably a good idea to put '10.1.x only' in the issue summary for ckeditor major updates.

I don't think we need to unpublish those releases, but we should do new ones. I'm short on time this afternoon so not sure I can get to those today.

catch’s picture

catch
he/him
Primary language English
commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Needs work » Fixed
Issue tags: -9.5.6 release notes, -10.0.6 release notes +10.1.0 release notes

Back to fixed for 10.1.0

@bradjones1 there's a minor release in June, so it's not long to wait for the new ckeditor5 major.

Status: Fixed » Closed (fixed)

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