Problem/Motivation

This should be done after #2911932: Correct vertical tab does not focus on form validation is committed so it is PP-1.

In #3261942: Compatibility issues with inline form errors we disabled inline form errors when a user tries to switch their editor to CKEditor 5. This was because if a user has invalid settings for CKEditor 5, the useful error message that tells the user what is wrong is not visible to the user when inline form errors is enabled.

This issue is to re-enable inline form errors by removing the workaround added after #2911932: Correct vertical tab does not focus on form validation is done.

Steps to reproduce

Proposed resolution

Remove the lines that disable inline form errors inside of
\Drupal\ckeditor5\Plugin\Editor\CKEditor5::assessActiveTextEditorAfterBuild

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3263668-6.patch1.05 KBwim leers

Issue fork drupal-3263668

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

hooroomoo created an issue. See original summary.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Title: Re-enable inline form errors in assessActiveTextEditorAfterBuild function » [PP-1] Re-enable inline form errors in assessActiveTextEditorAfterBuild function
Issue summary: View changes
Status: Active » Postponed
Related issues: +#3261942: Compatibility issues with inline form errors, +#2911932: Correct vertical tab does not focus on form validation

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

idebr’s picture

idebr commented
Title: [PP-1] Re-enable inline form errors in assessActiveTextEditorAfterBuild function » Re-enable inline form errors in assessActiveTextEditorAfterBuild function
Status: Postponed » Active

#2911932: Correct vertical tab does not focus on form validation is fixed, so this issue is no longer postponed.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
Issue tags: +Accessibility, +Needs manual testing
StatusFileSize
new 3263668-6.patch1.05 KB
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing +Needs Review Queue Initiative

Installed inline error forms
On a text format that was setup for ckeditor4
Changed to ckeditor5
Nothing seemed out of normal

Pointed out one of my ckeditor modules has no upgrade paths haha

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3263668-6.patch, failed testing. View results

mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue tags: +wcag331

Tagging for WCAG 3.3.1

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community

Unrelated failure in Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest::testConfigSync.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3263668-6.patch, failed testing. View results

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Novice

We should convert this to an MR — anybody who wants to do that? :) 🙏

omkar-pd made their first commit to this issue’s fork.

omkar-pd opened merge request !5618

omkar-pd’s picture

omkar-pd commented

Created MR from the patch #6.

omkar-pd’s picture

omkar-pd commented
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review

Thanks, @omkar-pd!

The following Nightwatch test failed:

Tests.ckEditor5EditorHeightTest: Ensure CKEditor respects field widget row value

Re-testing…

smustgrave’s picture

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

Reroll seems fine, does appear to be random failure.

quietone’s picture

quietone commented
Issue tags: -Novice

I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions or other work to do.

Leaving at RTBC.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Backported to 10.2.x as a tests-only fix.

Committed and pushed 86bd3d1299 to 11.x and 30c2899f4e to 10.2.x. Thanks!

  • longwave committed 30c2899f on 10.2.x 
    Issue #3263668 by omkar-pd, Wim Leers, hooroomoo: Re-enable inline form...

  • longwave committed 86bd3d12 on 11.x 
    Issue #3263668 by omkar-pd, Wim Leers, hooroomoo: Re-enable inline form...

Status: Fixed » Closed (fixed)

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