Test to prevent regression between quickedit and ckeditor [#2917218]

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.

Comment	File	Size	Author
#16	interdiff-3-15.txt	1.65 KB	Anonymous (not verified)
#15	2917218-15.patch	3.77 KB	Anonymous (not verified)
#15	8.5.x: PHP 7 & MySQL 5.5 23,378 pass
#3	2917218-3.patch	5.73 KB	Anonymous (not verified)
#3	8.5.x: PHP 7 & MySQL 5.5 22,391 pass
#3	2917218-3-test-only.patch	3.88 KB	Anonymous (not verified)
#3	8.5.x: PHP 7 & MySQL 5.5 22,390 pass, 2 fail 8.4.x: PHP 7 & MySQL 5.5 22,289 pass, 2 fail
#2	after_patch.png	17.58 KB	Anonymous (not verified)
#2	2917218-2.patch	1.85 KB	Anonymous (not verified)
#2	8.5.x: PHP 7 & MySQL 5.5 22,390 pass
	error_when_quickedit_body_field.png	24.67 KB	Anonymous (not verified)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Anonymous (not verified) CreditAttribution: Anonymous commented 18 October 2017 at 21:45

vaplas created an issue. See original summary.

Comment #2

Anonymous (not verified) CreditAttribution: Anonymous commented 18 October 2017 at 21:47

Status:

Active

» Needs review

File	Size
2917218-2.patch	1.85 KB
8.5.x: PHP 7 & MySQL 5.5 22,390 pass
after_patch.png	17.58 KB

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

fix error

Comment #3

Anonymous (not verified) CreditAttribution: Anonymous commented 20 October 2017 at 00:00

File	Size
2917218-3-test-only.patch	3.88 KB
8.5.x: PHP 7 & MySQL 5.5 22,390 pass, 2 fail 8.4.x: PHP 7 & MySQL 5.5 22,289 pass, 2 fail
2917218-3.patch	5.73 KB
8.5.x: PHP 7 & MySQL 5.5 22,391 pass

Comment #4

20 October 2017 at 00:51

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

Comment #5

Anonymous (not verified) CreditAttribution: Anonymous commented 30 October 2017 at 09:34

Issue tags:

+ckeditor

Comment #6

andypost

he/him

Russian

CreditAttribution: andypost as a volunteer and at Skilld commented 30 October 2017 at 17:10

3 files were hidden/shown/deleted

File	Size
error_when_quickedit_body_field.png	24.67 KB
2917218-2.patch	1.85 KB
8.5.x: PHP 7 & MySQL 5.5 22,390 pass
after_patch.png	17.58 KB

Looks good to go

Comment #7

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott as a volunteer and commented 3 November 2017 at 10:58

Priority:

Normal

» Critical

This is a critical bug as it got released in 8.4.1

Comment #8

cilefen CreditAttribution: cilefen as a volunteer commented 3 November 2017 at 11:00

I was just typing the same!

Comment #9

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott as a volunteer and commented 3 November 2017 at 11:03

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

Comment #10

Skin

Italian

Italy

CreditAttribution: Skin commented 3 November 2017 at 11:06

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.

Comment #11

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 3 November 2017 at 11:13

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

Comment #12

mlewand CreditAttribution: mlewand commented 3 November 2017 at 13:00

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.

Comment #13

cilefen CreditAttribution: cilefen as a volunteer commented 3 November 2017 at 17:30

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

Comment #14

Skin

Italian

Italy

CreditAttribution: Skin commented 3 November 2017 at 18:43

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

Comment #15

Anonymous (not verified) CreditAttribution: Anonymous commented 5 November 2017 at 10:46

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

File	Size
2917218-15.patch	3.77 KB
8.5.x: PHP 7 & MySQL 5.5 23,378 pass

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.

Comment #16

Anonymous (not verified) CreditAttribution: Anonymous commented 5 November 2017 at 10:17

File	Size
interdiff-3-15.txt	1.65 KB

Comment #17

5 November 2017 at 10:17

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.

Comment #18

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 27 March 2018 at 08:57

Status:

Needs review

» Reviewed & tested by the community

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

Comment #19

29 March 2018 at 14:58

lauriii committed 966dca1 on 8.6.x

Issue #2917218 by vaplas: Test to prevent regression between quickedit...

Comment #20

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 29 March 2018 at 14:58

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!