Problem/Motivation

We need to test the quickedit and ckeditor in an environment with a working javascript. Otherwise, we can get an elusive regression due to js-errors.

This can be closed as a duplicate, after #2828528: Add Quick Edit Functional JS test coverage

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Original post

Step to reproduce for last 8.5.x:
1. Choose 'Quick edit' for body field.
2. Click by text (or typing any symbol)
3. Bit wait and see Browser Console log (F12).
error
4. No CKEditor and 'Save' button.

CommentFileSizeAuthor
#16 interdiff-3-15.txt1.65 KBAnonymous (not verified)
#15 2917218-15.patch3.77 KBAnonymous (not verified)
#3 2917218-3.patch5.73 KBAnonymous (not verified)
#3 2917218-3-test-only.patch3.88 KBAnonymous (not verified)
#2 after_patch.png17.58 KBAnonymous (not verified)
#2 2917218-2.patch1.85 KBAnonymous (not verified)
error_when_quickedit_body_field.png24.67 KBAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented

vaplas created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 2917218-2.patch1.85 KB
new after_patch.png17.58 KB

Patch with https://github.com/ckeditor/ckeditor-releases/tree/master/plugins/shared... solved problem.

fix error

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2917218-3-test-only.patch3.88 KB
new 2917218-3.patch5.73 KB

The last submitted patch, 3: 2917218-3-test-only.patch, failed testing. View results

Anonymous’s picture

Anonymous (not verified) commented
Issue tags: +ckeditor
andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#2908864: Update CKEditor library to 4.7.3

Looks good to go

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Priority: Normal » Critical

This is a critical bug as it got released in 8.4.1

cilefen’s picture

cilefen commented

I was just typing the same!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I can confirm it for me.

If I do git revert -n defc9a0c9d on 4.7.1 then it is fixed. I think we should revert the CKEditor upgrade from 8.4.x and 8.5.x

skin’s picture

skin
Primary language Italian
Location Italy
commented

Thanks I` ve tested the patch on drupal 8.4.1 and now quick edit works as expected and the Browser console is clean from errors.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

WTF, how did that happen?! Pinging the CKEditor team.

mlewand’s picture

mlewand commented

Looks like the plugin (sharedspace) is simply missing. Assuming that you're not bundling it into the main ckeditor.js file it should be available in `ckeditor/plugins/sharedspace` directory.

I'm trying to set up D8 instance based on 8.5.x branch to see what exactly is going on.

cilefen’s picture

cilefen commented

Try 8.4.2: https://www.drupal.org/project/drupal/releases/8.4.2

skin’s picture

skin
Primary language Italian
Location Italy
commented

I` ve just tried Drupal 8.4.2 ad quick edit is working.

Anonymous’s picture

Anonymous (not verified) commented
Title: When quickediting the body field error 'Resource name "sharedspace" was not found' » Test to prevent regression between quickedit and ckeditor
Priority: Critical » Normal
Issue summary: View changes
Related issues: +#2828528: Add Quick Edit Functional JS test coverage, +#2870458: Convert web tests to browser tests for quickedit module
StatusFileSize
new 2917218-15.patch3.77 KB

So, now it is not critical issue, because after the lightning reaction, we have drupal 8.4.2 without this flaw. And an alerted CKEditor team. Awesome!

What now? Based on #2908864-15: Update CKEditor library to 4.7.3 we need more reliable JSB testing.

We already have issue about #2870458: Convert web tests to browser tests for quickedit module, after which the tests will definitely be better. But we are unlikely to add an additional test coverage there, if it goes beyond the existing code logic.

Therefore, we can add advanced checks in current issue (with update title + IS), without waiting for the convert issue.

I began to move in this direction, when I suddenly found #2828528: Add Quick Edit Functional JS test coverage!

A year ago, @Wim Leers invested in this task, and together with other developers created a super thorough test coverage!

So, I just slightly corrected my patch, because in CKEditor 4.7.2 extra line breaks are added. Next I will try to push forward #2828528: Add Quick Edit Functional JS test coverage. If it is successful, I also closed current issue as a duplicate.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new interdiff-3-15.txt1.65 KB

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

wim leers’s picture

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

How did I not see #15?! :( Sorry, @vaplas!

  • lauriii committed 966dca1 on 8.6.x 
    Issue #2917218 by vaplas: Test to prevent regression between quickedit...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Thank you @vaplas. Adding this test case is a good step to the right direction and will make sure that the integration between Quickedit and CKEditor isn't completely broken.

Committed 966dca1 and pushed to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

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