Problem/Motivation

https://github.com/ckeditor/ckeditor5/releases/tag/v43.3.1

Steps to reproduce

Proposed resolution

Remaining tasks

Backport this to the gazillion current branches, TBD by the core committers.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#22 3487579-nr-bot.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3487579

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

spokje created an issue. See original summary.

spokje opened merge request !10190

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review

Updated @ckeditor/*, ckeditor5 and webpack to their latest releases.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
spokje’s picture

spokje
Primary language Dutch
commented
Priority: Normal » Critical
Issue summary: View changes

Mirroring status of parent issue, which became critical.

smustgrave’s picture

smustgrave commented

Do we add test coverage for new features added?

spokje’s picture

spokje
Primary language Dutch
commented

Do we add test coverage for new features added?

AFAICT we never did with previous bumps. although the idea is not without its merits.

I fear however, if we did, we'd be about just finished with defining and implementing the tests when a new version comes out.

Anyway: n=1, mileage may vary, yada yada.

smustgrave’s picture

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

The one I was curious about was 43.3.0 with media-embed but now realize we don't have that button lol.

Applied the update locally at least and ckeditor seems to behave as expected.

  • catch committed 81866151 on 11.1.x 
    Issue #3487579 by spokje, smustgrave: Bump ckeditor 43.1.1 => 43.3.1

(...

  • catch committed c07017bb on 11.x 
    Issue #3487579 by spokje, smustgrave: Bump ckeditor 43.1.1 => 43.3.1

catch closed merge request !10190

catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.5.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 11.x and 11.1.x, thanks!

We should backport this to 10.5.x/10.4.x so moving there.

spokje changed the visibility of the branch 11.x to hidden.

spokje opened merge request !10231

spokje’s picture

spokje
Primary language Dutch
commented
Status: Patch (to be ported) » Needs review
smustgrave’s picture

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

Seems like a good re-roll for 10.5

catch’s picture

catch
he/him
Primary language English
commented

I'm getting errors from commit-code.sh trying to commit this locally.

yarn check -s says:

error "acorn" is wrong version: expected "8.12.1", got "8.11.3"
warning Resolution field "ejs@3.1.10" is incompatible with requested version "nightwatch#ejs@3.1.8"
warning Resolution field "nightwatch#semver@7.5.4" is incompatible with requested version "nightwatch#semver@7.3.5"
warning "postcss-preset-env#autoprefixer#caniuse-lite@^1.0.30001599" could be deduped from "1.0.30001664" to "caniuse-lite@1.0.30001664"
error "espree#acorn" not installed
error "espree#acorn-jsx" not installed
error Found 3 errors.

This might be just an issue with my local so leaving RTBC.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Needs work

Same happens here. I think it has something to do with the fact I mistakenly started with a $ $ yarn upgrade-interactive, ehich is more/only for yarn 4.x

Let's start with a new MR.

spokje closed merge request !10231

spokje opened merge request !10232

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review

Well, that required a lot more elbow grease than I expected/needed, but the errors are gone.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3487579-nr-bot.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

No, bad bot!

spokje’s picture

spokje
Primary language Dutch
commented
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work

hang on, yarn vendor-update was not run on 11.x, lots of new languages were added. So we're missing translation files

spokje’s picture

spokje
Primary language Dutch
commented

hang on, yarn vendor-update was not run on 11.x, lots of new languages were added. So we're missing translation files

yarn build was run on 11.x here:
https://git.drupalcode.org/project/drupal/-/merge_requests/10190/diffs?c...

yarn build includes yarn vendor-update.

So I'm confused now, what files are missing and do they turn up when you do a yarn build on 11.x?
Do the turn up if you do a yarn vendor-update on 11.x?

If the first question is answered with a yes, I messed it up.
Anything else, I'm scratching my head.

spokje’s picture

spokje
Primary language Dutch
commented

Discussed this with @nod_ in Slack, and, of course he was right.

Adding an MR with the MIA files now.

spokje opened merge request !10246

spokje’s picture

spokje
Primary language Dutch
commented
Version: 10.5.x-dev » 11.x-dev
Status: Needs work » Needs review

Opened MR!10246 to add the missing JS files to 11.x, hoping that it's a 1-on-1 backport to 11.1.x.

I've changed my local, faulty .gitignore so this won't happen again.

All credits for finding out about this distaster-in-progress to @nod_!

  • nod_ committed 695911c0 on 11.1.x 
    Issue #3487579 by spokje, smustgrave, catch, nod_: Bump ckeditor 43.1.1...

  • nod_ committed e116e045 on 11.x 
    Issue #3487579 by spokje, smustgrave, catch, nod_: Bump ckeditor 43.1.1...
spokje’s picture

spokje
Primary language Dutch
commented

Added missing files to the 10.5.x MR, but that branch seems to have a broken HEAD: https://www.drupal.org/project/drupal/issues/3488179#comment-15863742

nod_ closed merge request !10246

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.5.x-dev

Thanks!

spokje’s picture

spokje
Primary language Dutch
commented

HEAD unbroken, rebased

smustgrave’s picture

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

Rebase seems good on 10.5.x now.

  • catch committed 41f0ccb5 on 10.4.x 
    Issue #3487579 by spokje, smustgrave, catch, nod_: Bump ckeditor 43.1.1...

catch closed merge request !10232

  • catch committed 4ffbaeb6 on 10.5.x 
    Issue #3487579 by spokje, smustgrave, catch, nod_: Bump ckeditor 43.1.1...
catch’s picture

catch
he/him
Primary language English
commented
Version: 10.5.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 10.5.x and cherry-picked to 10.4.x, thanks!

I'm leaving this open for discussion about whether we want to try to update 10.3.x too - it might make things easier if there's a ckeditor5 security release in the next six months, if the update is otherwise non-disruptive.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I don't think we should, just because there were no breaking changes in v42 or v43 that affected core doesn't mean that contrib or custom plugins won't be affected.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 10.3.x-dev » 10.4.x-dev
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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