Issue fork drupal-3587182

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:

Comments

joseph.olstad created an issue. See original summary.

longwave’s picture

Status: Active » Postponed

CKEditor5 v48.0.0 no longer supports the DLL installation method: https://github.com/ckeditor/ckeditor5/releases/tag/v48.0.0

That means this issue is postponed on #3527914: Use UMD installation method for CKEditor5

Please note also that this is unlikely to land in Drupal 11; per our agreement with CKSource we intend to stay on the LTS v47 version of CKEditor5 in Drupal 11.

longwave’s picture

joseph.olstad’s picture

Ok, gotcha, thats fine since we have the ckeditor5_table_fix module that will get us through until then.

Now we know what to expect! Sounds good!

godotislate’s picture

Status: Postponed » Active

#3527914: Use UMD installation method for CKEditor5 is in, so we can go ahead for D12 (only).

andrew.wang’s picture

Issue summary: View changes

Re-formatted issue summary with nested lists.

longwave’s picture

Status: Active » Needs review

godotislate’s picture

One comment on the MR.

Also, we haven't done #3551053: Remove jsdoc integration for CKEditor5, but I wonder if the script even works anymore with the new build.

longwave’s picture

Updated the deprecated versions, I think we should keep them in sync. yarn vendor-update no longer updates them for us, so did it manually with sed.

The jsdoc script doesn't work, it just deletes all entries in the file, so I left that alone for that other issue.

longwave’s picture

Fixed the jsdoc script, the glob and regex just need updating a tiny bit to look at the TypeScript definitions. I closed #3551053: Remove jsdoc integration for CKEditor5.

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

Comments all addressed, lgtm.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new91 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.

godotislate’s picture

I rebased to make sure, but a test failure persist:

Drupal\Tests\demo_umami\FunctionalJavascript\AssetAggregationAcrossPagesTest::testAssetsAcrossMultiplePages
Asserting ScriptBytes 3206361 is greater or equal to 3222037 and is smaller or equal to 3223037
Failed asserting that 3206361 ( is equal to 3222037 or is greater than 3222037 ) and ( is equal to 3223037 or is less than 3223037 ).

The cron one seems to be intermittent.

godotislate’s picture

Status: Needs work » Reviewed & tested by the community

I fixed the test byte numbers in AssetAggregationAcrossPagesTest. Since this was RTBC before, and everything since is merge conflict resolution or performance test tweaks, I'm going to put this back to RTBC.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new40.2 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

godotislate’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: +no-needs-review-bot

Silencing the bot because it's flagging a bunch of false eslint issues like:

core/modules/ckeditor5/js/ckeditor5_plugins/drupalEmphasis/src/drupalemphasis.js
  4:24  error  Unable to resolve path to module 'ckeditor5/src/core'  import/no-unresolved
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 30521cd6 on main
    task: #3587182 Update CKEditor 5 to 48.0.1
    
    By: joseph.olstad
    By:...

Status: Fixed » Closed (fixed)

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