Problem/Motivation

We are committing lots of CKEditor 5 changes and every time we commit a change, we have to manually ensure that the files have been built correctly, using correct dependencies. This is also potential security concern, since if the committer forgets this step, it could potentially allow contributors (either accidentally or intentionally) to inject malicious code without anyone noticing.

Proposed resolution

Add step for DrupalCI that runs yarn build:ckeditor5 for the CKEditor 5 built files when they are being changed, and ensures that there aren't any changes after running the command.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3267721

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

lauriii created an issue. See original summary.

nod_’s picture

without looking too deeply into it, seems 100% related to #3238497: What to do with assets build step? and what sally proposed, that drupal ci does the compile step somehow.

lauriii’s picture

Totally related to that one 👍 Just noting that since we will need the build step for the lifecycle of Drupal 9 at minimum, it would still be worth while adding a step to check this regardless of what we do there.

nod_’s picture

Version: 9.4.x-dev » 10.0.x-dev
nod_’s picture

ugh, bash… too young for this :p I'll get back to it next week

wim leers’s picture

nod_’s picture

Status: Active » Needs review
wim leers’s picture

Manually tested, works great!

Tested:

  1. no changes, just this MR:
    $ yarn run -s check:ckeditor5
    $ echo $?
    0
    
  2. made this change:
    diff --git a/core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediametadatarepository.js b/core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediametadatarepository.js
    index fc023b6cb8..ca8c37b0be 100644
    --- a/core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediametadatarepository.js
    +++ b/core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediametadatarepository.js
    @@ -23,7 +23,7 @@ const _fetchMetadata = async (url) => {
         return JSON.parse(await response.text());
       }
     
    -  throw new Error('Fetching media embed metadata from the server failed.');
    +  throw new Error('Fetching media embed metadata from the server failed dramatically.');
     };
     
     /**
    
  3. $ yarn run -s check:ckeditor5
    $ echo $?
    1
    

Once the nits are addressed, this is RTBC!

nod_’s picture

updated the comments

wim leers’s picture

Status: Needs review » Reviewed & tested by the community
wim leers’s picture

Re-confirming RTBC :)

  • lauriii committed da12af1 on 10.0.x
    Issue #3267721 by nod_, Wim Leers: Add DrupalCI step for ensuring that...

  • lauriii committed 4753c4f on 9.4.x
    Issue #3267721 by nod_, Wim Leers: Add DrupalCI step for ensuring that...
lauriii’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed da12af1 and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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