Problem/Motivation

CKEditor 4 will be removed from core in Drupal 10.

The Inline Form Errors module has test coverage to ensure it works well with both CKEditor 4 and CKEditor 5:

  • Drupal\Tests\inline_form_errors\FunctionalJavascript\FormErrorHandlerCKEditorTest
  • Drupal\Tests\inline_form_errors\FunctionalJavascript\FormErrorHandlerCKEditor5Test

There does not appear to be an integration code inside Inline Form Errors itself (no references to ckeditor, case-insensitive, outside the two tests).

Proposed resolution

Move the CKEditor 4 test coverage to the ckeditor namespace.

Leave Drupal\Tests\inline_form_errors\FunctionalJavascript\FormErrorHandlerCKEditor5Test in place for testing CKEditor 5 integration.

Remaining tasks

TBD

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3271046

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue summary: View changes

xjm opened merge request !2004

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Active » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

Given we are changing the namespace from IFE to CKEditor, should we remove CKEditor from the test class name and change it to something like InlineFormErrorsTest?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

@longwave Personally I think it's fine as-is; the name already included reference to both modules. I always find that easier to understand anyway.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
CreditAttribution: danflanagan8 at Horizontal commented
Status: Needs review » Reviewed & tested by the community

Stumbled upon this issue as I was preparing to update inline_form_errors as part of the classy-to-stark meta. I thought maybe there'd be some issue about FormErrorHandlerCKEditorTest and here we are! This appears to be the only test in inline_form_errors that would need to be fixed. So this move kills two birds with one stone!

Confirmed that ckeditor is not mentioned outside the two tests mentioned. Looks good to me.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

  • catch committed 8a5808c on 10.0.x 
    Issue #3271046 by xjm: Move integration test for CKEditor 4 and Inline...
  • catch committed 0bd2e3e on 9.4.x
    Issue #3271046 by xjm: Move integration test for CKEditor 4 and Inline...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed

Fixing status from bot crosspost or whatnot.

Status: Fixed » Closed (fixed)

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