Add missing CKE5 SmartDefaultSettings test coverage (wildcard tag with unsupported attribute) [#3259174]

Problem/Motivation

Add missing test coverage: CKE5's SmartDefaultSettings in the "supported tags but unsupported attributes" case works, but has zero test coverage.

Discovered this while working on #3228334: Refactor HTMLRestrictionsUtilities to a HtmlRestrictions value object. This issue blocks that! This is test coverage we forgot to introduce in #3216015: Generate CKEditor 5 configuration based on pre-existing text format configuration.

Steps to reproduce

N/A

Proposed resolution

Expand test coverage.

Note that we specifically do not attempt to improve the upgrade path here, only to add test coverage to prove that existing code works reasonably well.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

Comment	File	Size	Author
#16	3259174-16-9.3.x.patch	5.44 KB	Wim Leers
#16	9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass
#16	3259174-16-9.4.x_and_10.0.x.patch	5.44 KB	Wim Leers
#16	9.4.x: PHP 8.0 & MySQL 5.7 29,354 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,309 pass
#16	interdiff.txt	2.28 KB	Wim Leers
#12	3259174-12-9.3.x.patch	5.34 KB	hooroomoo
#12	9.3.x: PHP 8.0 & MySQL 5.7 29,228 pass, 1 fail
#11	3259174-11-9.4.x_10.0.x.patch	5.33 KB	hooroomoo
#11	9.4.x: PHP 8.0 & MySQL 5.7 29,343 pass, 1 fail 10.0.x: PHP 8.0 & MySQL 5.7 29,298 pass, 1 fail
#10	3259174-6-9.3.x.patch	5.34 KB	hooroomoo
#10	9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
#10	3259174-6-9.4.x_and_10.0.x.patch	5.33 KB	hooroomoo
#10	9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
#9	3259174-6-9.3.x.patch	5.33 KB	hooroomoo
#9	9.4.x: PHP 8.0 & MySQL 5.7 CI aborted 9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
#9	3259174-6-9.4.x_and_10.0.x.patch	5.33 KB	hooroomoo
#9	9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
#6	3259174-6-9.3.x.patch	5.31 KB	Wim Leers
#6	9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass
#6	3259174-6-9.4.x_and_10.0.x.patch	5.31 KB	Wim Leers
#6	9.4.x: PHP 8.0 & MySQL 5.7 29,344 pass, 1 fail 10.0.x: PHP 8.0 & MySQL 5.7 29,304 pass
#4	3259174-4-9.3.x.patch	5.32 KB	Wim Leers
#4	9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass
#3	interdiff.txt	291 bytes	Wim Leers
#6	interdiff.txt	1.27 KB	Wim Leers
#3	3259174-3.patch	5.32 KB	Wim Leers
#3	9.4.x: PHP 8.0 & MySQL 5.7 29,349 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,258 pass 9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
#2	3259174-2.patch	5.04 KB	Wim Leers
#2	9.4.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed 9.3.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed 10.0.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed
#4	3259174-4-9.4.x_and_10.0.x.patch	5.32 KB	Wim Leers
#4	9.4.x: PHP 8.0 & MySQL 5.7 29,349 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,304 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

18 January 2022 at 16:23

Wim Leers created an issue. See original summary.

Comment #2

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 18 January 2022 at 16:38

Status:

Active

» Needs review

File	Size
3259174-2.patch	5.04 KB
9.4.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed 9.3.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed 10.0.x: PHP 8.0 & MySQL 5.7 Custom Commands Failed

File

Size

3259174-2.patch

5.04 KB

9.4.x:
PHP 8.0 & MySQL 5.7 Custom Commands Failed

9.3.x:
PHP 8.0 & MySQL 5.7 Custom Commands Failed

10.0.x:
PHP 8.0 & MySQL 5.7 Custom Commands Failed

Comment #3

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 18 January 2022 at 16:53

File	Size
interdiff.txt	291 bytes
3259174-3.patch	5.32 KB
9.4.x: PHP 8.0 & MySQL 5.7 29,349 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,258 pass 9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply

File

Size

interdiff.txt

291 bytes

3259174-3.patch

5.32 KB

9.4.x:
PHP 8.0 & MySQL 5.7 29,349 pass

10.0.x:
PHP 8.0 & MySQL 5.7 29,258 pass

9.3.x:
PHP 8.0 & MySQL 5.7 Patch Failed to Apply

Comment #4

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 19 January 2022 at 07:13

File	Size
3259174-4-9.3.x.patch	5.32 KB
9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass
3259174-4-9.4.x_and_10.0.x.patch	5.32 KB
9.4.x: PHP 8.0 & MySQL 5.7 29,349 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,304 pass

The patch in #3 did not apply to 9.3.x, only due to the dictionary being different in 9.3.x.

Here's the same patch uploaded again for 9.4.x and 10.0.x and a different patch for 9.3.x.

Comment #5

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 19 January 2022 at 13:27

+++ b/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php
@@ -456,6 +468,55 @@ public function provider() {
+        // @todo Avoid allowing alignment on other tags, blocked on upstream: https://github.com/ckeditor/ckeditor5/issues/11131

I think we should open an issue in our issue queue to track the upstream issue, and link it here.

Comment #6

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 19 January 2022 at 13:47

File	Size
interdiff.txt	1.27 KB
3259174-6-9.4.x_and_10.0.x.patch	5.31 KB
9.4.x: PHP 8.0 & MySQL 5.7 29,344 pass, 1 fail 10.0.x: PHP 8.0 & MySQL 5.7 29,304 pass
3259174-6-9.3.x.patch	5.31 KB
9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass

Done: #3259367: [upstream] Allow configuring *which* HTML tags can be aligned.

Comment #7

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 20 January 2022 at 11:41

Status:

Needs review

» Reviewed & tested by the community

Looks good, thank you!

Comment #8

bnjmnm

he/him

English

Ann Arbor, MI

CreditAttribution: bnjmnm at Acquia commented 21 January 2022 at 17:34

Status:

Reviewed & tested by the community

» Needs work

+++ b/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php
@@ -456,6 +468,55 @@ public function provider() {
+        // @todo Improve this in https://www.drupal.org/project/drupal/issues/3231328
+        '<p class="text-align-left text-align-right">',
+        // Note that aligning left/center/right/justify is possible on *all*
+        // allowed block-level HTML5 tags.
+        // @todo When https://www.drupal.org/project/ckeditor5/issues/3231328 lands, only the center/justify classes will be added.
+        // @todo When https://www.drupal.org/project/drupal/issues/3259367 lands, none of the tags below should appear.

This is such a nit that my dog is concerned about my visible embarrassment. These three @todo items exceed the 80 char limit. They should be extended to additional lines with a two space indent to indicate they're a continuation of the @todo

Comment #9

hooroomoo

they/them

CreditAttribution: hooroomoo at Acquia commented 21 January 2022 at 18:20

Status:

Needs work

» Needs review

File	Size
3259174-6-9.4.x_and_10.0.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
3259174-6-9.3.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 CI aborted 9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply

Uploaded same patches in #6 with added indents

Comment #10

hooroomoo

they/them

CreditAttribution: hooroomoo at Acquia commented 21 January 2022 at 19:05

File	Size
3259174-6-9.4.x_and_10.0.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
3259174-6-9.3.x.patch	5.34 KB
9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply

Comment #11

hooroomoo

they/them

CreditAttribution: hooroomoo at Acquia commented 21 January 2022 at 19:35

File	Size
3259174-11-9.4.x_10.0.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 29,343 pass, 1 fail 10.0.x: PHP 8.0 & MySQL 5.7 29,298 pass, 1 fail

File

Size

3259174-11-9.4.x_10.0.x.patch

5.33 KB

9.4.x:
PHP 8.0 & MySQL 5.7 29,343 pass, 1 fail

10.0.x:
PHP 8.0 & MySQL 5.7 29,298 pass, 1 fail

Comment #12

hooroomoo

they/them

CreditAttribution: hooroomoo at Acquia commented 21 January 2022 at 19:48

File	Size
3259174-12-9.3.x.patch	5.34 KB
9.3.x: PHP 8.0 & MySQL 5.7 29,228 pass, 1 fail

4 files were hidden/shown/deleted

File	Size
3259174-6-9.4.x_and_10.0.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
3259174-6-9.3.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 CI aborted 9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
3259174-6-9.4.x_and_10.0.x.patch	5.33 KB
9.4.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply 10.0.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply
3259174-6-9.3.x.patch	5.34 KB
9.3.x: PHP 8.0 & MySQL 5.7 Patch Failed to Apply

Comment #13

21 January 2022 at 20:37

The last submitted patch, 11: 3259174-11-9.4.x_10.0.x.patch, failed testing. View results

Comment #14

21 January 2022 at 20:54

Status:

Needs review

» Needs work

The last submitted patch, 12: 3259174-12-9.3.x.patch, failed testing. View results

Comment #15

bnjmnm

he/him

English

Ann Arbor, MI

CreditAttribution: bnjmnm at Acquia commented 21 January 2022 at 21:25

+++ b/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php
@@ -456,6 +468,57 @@ public function provider() {
+            // @todo Improve in https://www.drupal.org/project/drupal/issues/3259179

Issue 3259179 landed earlier today, the changes introduced there makes this test outdated as the alignment operations have been split into individual plugins so enabling one does not automatically enable the others.

Comment #16

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 24 January 2022 at 12:38

Status:	Needs work	» Reviewed & tested by the community
Related issues:		+#3259179: Split ckeditor5_alignment CKEditor 5 plugin, to allow for more precise upgrade path, +#3259593: Alignment being available as separate buttons AND in dropdown is confusing

File	Size
interdiff.txt	2.28 KB
3259174-16-9.4.x_and_10.0.x.patch	5.44 KB
9.4.x: PHP 8.0 & MySQL 5.7 29,354 pass 10.0.x: PHP 8.0 & MySQL 5.7 29,309 pass
3259174-16-9.3.x.patch	5.44 KB
9.3.x: PHP 8.0 & MySQL 5.7 29,238 pass

This is such a nit that my dog is concerned about my visible embarrassment.

🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣🤣

Rebased now that #3259179: Split ckeditor5_alignment CKEditor 5 plugin, to allow for more precise upgrade path is in 👍 And now we have #3259593: Alignment being available as separate buttons AND in dropdown is confusing, which is the next way that this test coverage can be refined.

Comment #17

24 January 2022 at 15:30

bnjmnm committed 0466cba on 9.3.x

Issue #3259174 by Wim Leers, lauriii, hooroomoo, bnjmnm: Add missing...

Comment #18

24 January 2022 at 15:30

bnjmnm committed 0722b46 on 10.0.x

Issue #3259174 by Wim Leers, lauriii, hooroomoo, bnjmnm: Add missing...

Comment #19

24 January 2022 at 15:31

bnjmnm committed 9df7f5a on 9.4.x

Issue #3259174 by Wim Leers, lauriii, hooroomoo, bnjmnm: Add missing...

Comment #20

bnjmnm

he/him

English

Ann Arbor, MI

CreditAttribution: bnjmnm at Acquia commented 24 January 2022 at 15:32

Status:

Reviewed & tested by the community

» Fixed

This is now current with #3259179: Split ckeditor5_alignment CKEditor 5 plugin, to allow for more precise upgrade path so it's merged to 9.4.x/10.0.x as well as 9.3.x since CKEditor 5 is experimental and this is a non-disruptive expansion of test coverage.