Research if the CKE off-canvas CSS reset could be optimized

I think the "formal" solution will be possible with CSS cascade layers (see https://www.w3.org/2021/10/TPAC/demos/css-architecture.html and https://www.bram.us/2021/09/15/the-future-of-css-cascade-layers-css-at-l...)

If layers are available, it also mean no shadow dom necessary for things like #2195695: Admin UIs on the front-end are difficult to theme

TIL cascade layers 😮

Not yet widely available though: https://caniuse.com/css-cascade-layers. Only next versions of Chrome, Firefox and Safari, i.e. 0.1% of today's browser usage 😬

In the current code the styles reset result change depending on aggregation being activated or not.

When CSS aggregation is ON, there are 0 rules ignored.

When CSS aggregation is OFF, 237 rules from those css files are ignored:

/core/misc/dialog/off-canvas.reset.css
/core/misc/dialog/off-canvas.base.css
/core/misc/dialog/off-canvas.css
/core/misc/dialog/off-canvas.theme.css
/core/misc/dialog/off-canvas.motion.css
/core/misc/dialog/off-canvas.button.css
/core/misc/dialog/off-canvas.form.css
/core/misc/dialog/off-canvas.table.css
/core/misc/dialog/off-canvas.details.css
/core/misc/dialog/off-canvas.tabledrag.css
/core/misc/dialog/off-canvas.dropbutton.css
/core/misc/dialog/off-canvas.layout.css

I … am not sure I fully understand that. Are you saying you found a bug in HEAD?

I guess, but it's not really important since the extra 200 rules generated will never match anything it turns #drupal-off-canvas * into #drupal-off-canvas [data-drupal-ck-style-fence] #drupal-off-canvas *.

Got a better solution using donut selectors: https://css-tricks.com/weekly-platform-news-focus-rings-donut-scope-ditc...

You keep throwing new terms at me — donut selector this time 🤪

Also: wow, that looks super interesting.

I looked at the MR, and I think @bnjmnm would be a far better reviewer than I possibly could be. He has recent/prior experience in this area, I don't. To me, what jumps out, are coding standards nitpicks (incomplete JS docblocks).

Also previous code seems like it doesn't work for rules inside media queries or rtl styles (that have [dir=rtl] #drupal-off-canvas for example)

That seems like a pretty significant bug.

Which begs the question … are we missing test coverage? 😬 (That sounds like no fun to write test coverage for, and I'm not sure it's worth it, but I have no choice but to ask.)

For the tests it's the css that will be annoying to write. Should be easy enough overall though.

i do think we should at least find the cause of those issues and add a few comment in the code in this issue, even if we don't solve everything.

Does the "donut scope" selector work in IE11? We had issues with complex :not() before, see #2958588: Off-canvas style resets are overriding styles (especially SVGs) resulting in display issues and https://caniuse.com/css-not-sel-list

If it doesn't, this will have to be Drupal 10 only.

good question, not sure if [attr] * is considered a selector list (probably i'd say).

As far as core support goes, it's experimental in 9.4 so we might get away with it? no issue for D10 though, that's good. Can't be a generic solution but can be suitable for CKE depending on the answer to getting away with it in experimental

So during testing we can see that the previous code was working and it worked because of the increased specificity of the cke selectors

it turned .cke.cke-content into #drupal-off-canvas [data-drupal-ck-style-fence] .cke.cke-content making that rule apply above all the others. It also turns @media screen, print {} into #drupal-off-canvas [data-drupal-ck-style-fence] @media screen, #drupal-off-canvas [data-drupal-ck-style-fence] print {}, the rule is invalid and gets ignored so no harm. We could avoid the unnecessary work though.

With the patch the styles are not applied and there is nothing to override. It doesn't break media queries or nested media, supports declarations.

I don't have a failing test though.

Tests work locally with chrome 97 and in core we have chrome 87, which doesn't support the selector: https://caniuse.com/css-not-sel-list

in core we have chrome 87

We should update core. Chrome 87 is from November 2020: https://developer.chrome.com/blog/new-in-chrome-87/.

Per https://caniuse.com/usage-table, a tiny fraction uses Chrome 87.

Also, per core/package.json:

  "browserslist": [
    "last 2 Chrome major versions",
    …

But, important to ask: which is the first version of Chrome that supports this selector?

version 88 :D

That seems … reasonable :D

I think we need a follow-up issue that is postponed on #3258114: Upgrade Chromedriver (thanks for that!) to bring back the extra tests?

going to post the tests in #3259769: Add tests for CKEditor 5 off-canvas CSS reset

👍

I don't think I'm eligible to RTBC this, so keeping at Needs review — I've done all I can to push this one forward 😊

Added more comments. I'm really not sure how best to explain it :/ let me know if that helps.

one more thing that the script does, duplicate all .ck* css rules and prefix them with #drupal-off-canvas [data-drupal-ck-style-fence].

Before

.ck-content .table > figcaption {}
[dir="ltr"] .ck.ck-input-color .ck.ck-input-color__remove-color {}

After

.ck-content .table > figcaption, 
#drupal-off-canvas [data-drupal-ck-style-fence] .ck-content .table > figcaption {}

[dir="ltr"] .ck.ck-input-color .ck.ck-input-color__remove-color, 
[dir="ltr"] #drupal-off-canvas [data-drupal-ck-style-fence] .ck.ck-input-color .ck.ck-input-color__remove-color {}

2 random fails in media tests — re-testing.

Back to NW for docs-related things and to confirm one still-open @lauriii thread is addressed. Overall a very nice solution.

Changes look good. Could we get a Drupal 10 patch of the current MR?

Committed to 10.0.x, waiting for rebase to pass to merge MR to 9.4.x. Very elegant optimization!

This is probably a good candidate for 9.3 as CKEDitor 5 is experimental and the change is non disruptive. Will check with @lauriii before making it official, though.

+1

Cherry picked to 9.3.x