Create test for theme asset decoupling from Stable and address any regressions that decoupling may cause

This is the first of several comments reviewing how a single asset is used by each core theme.

block.admin.css

Impacts: Claro, Bartik, Seven, Umami
What is different: adds this rule

.block-disabled:not(:hover) {
  opacity: 0.675;
  background: #fcfcfa;
}

Which makes disabled blocks semi-transparent.

Claro

Using stable

Using core

The contrast meets AAA - 5.29:1.

Seven

Using stable

Using core

The contrast meets AA - 4.6:1

Bartik

Using stable

Using core

👿 Contrast is not accessible - 4.15:1

Umami

Using stable

Using core

👿Contrast is not accessible - 3.82:1

filter.caption.css

Impacts: Claro, Bartik, Umami, Seven
What is different: Stable adds this rule:

/* Undo margin added to figure in core/assets/vendor/normalize-css/normalize.css */
figure.caption-drupal-media.align-center {
  margin-right: auto;
  margin-left: auto;
}

This rule is unnecessary once #2821525: Update normalize.css to the most recent version lands, so this will not need to be changed unless the Normalize.css issue has a surprise block.

progress.module.css

Impacts: None
What is different in core vs. stable:

  .progress__bar {
     width: 3%;
     min-width: 3%;
-   height: 1.5em;
+  height: 16px;
    background-color: #000;
  }

No core themes are impacted as each have their own stylesheets that specify height for .progress__bar

system-modules-details.html.twig, system-modules-uninstall.html.twig

Impacts: none
What is different in core vs. stable:
An unnecessary data-striping attribute is removed in the core version, which happened in #2923305: Module (un-)install tables have a useless "data-striping" attribute

views-ui-views-listing-table.html.twig

Impacts: Umami, Seven, Bartik
What is different in core vs. stable:
Core implements the fix in #3059872: View names on view list table shouldn't be headings, which changes an <h3> that shouldn't be a heading to be wrapped in <strong> instead.
This is an acceptable/welcome change so this asset can be inherited from core.

container-inline.module.css

Impacts: Bartik, Umami
What is different in core vs. stable: .container-inline div, .container-inline label changed from display: inline; in stable to display: inline-block;
Changed in #2226317: Divs in the container-inline wrapper should be inline-block instead of inline

Umami

👿Regression 👿Tested with datetime input as that is one of the more likely elements to appear in non-admin context. There may be additional regressions but just this demonstrates that Umami can't inherit this style from core.

Bartik

👿Regression 👿
Datetime results in extra margin

Search input takes submit and input out of alignment

install-page.html.twig

Impacts: Bartik, Umami
What is different in core vs. stable: Core has the change implemented in #2528420: Two H1's in Installation Process, which addresses multiple <h1> elements on the install page. It is fine for Bartik and Umami to inherit this fix.

status-report-counter.html.twig

Impacts: Umami, Bartik
What is different in core vs. stable:
Core adds classes and icons

-{{ amount }} {{ text }}
+<span class="system-status-counter__status-icon system-status-counter__status-icon--{{ severity }}"></span>
+<span>{{ amount }} {{ text }}</span>
 <a href="#{{ severity }}"><span class="visually-hidden">{{ text }} </span>Details</a>

Bartik

From Stable

From Core

Umami

From Stable

From Core

In both Umami and Bartik, the only difference is the text is moved slightly to the right to make space for theoretical icons. In my opinion this is an acceptable change as 1) the text wasn't perfectly centered in the first place 2) This is a small change that doesn't "break" anything, and it appears on an admin element on a non-admin theme so it's very unlikely the template would ever appear in those themes.

status-report-general-info.html.twig

Impacts: Umami, Bartik
What is different in core vs. stable: A single char of whitespace
<h4>{{ 'Version'|t }}</h4> {{ php.value }}
VS
<h4>{{ 'Version'|t }}</h4>{{ php.value }}

This is an acceptable change. It does not impact functionality and would have been a good candidate for the scope of #3111729: Stable templates/CSS that differ only in comments or indentation should match core's version.

system-status-counter.html.twig

Impacts: Bartik, Umami
What is different in core vs. stable:
Changes to icons
.system-status-counter__status-icon:before {
display: block;
+ width: 100%;
+ height: 100%;
content: "";
background-repeat: no-repeat;
background-position: center 2px;
- background-size: 20px;
+ background-size: 16px;
}
Expanding on the conclusion made for status-report-counter.html.twig in #12, inheriting from core should be acceptable. The only change is width/height/background-size of an element that doesn't appear in these themes.

Based on reviewing each asset that differs in core / stable, I've sorted them into 4 piles:

Additional work definitely needed

• container-inline.module.css #10 (evaluation) Needed in Bartik and Umami. Created followup for additional steps to take after the basic decoupling: #3115710: Investigate changing container-inline rules in Umami

Additional work maybe needed

• block.admin.css #5 (evaluation). Insufficient contrast in Bartik and Umami. However, it is unlikely that these themes would display the Block admin UI as these are not admin themes. Would like additional input on whether this is acceptable or not.
• status-report-counter.html.twig #12 (evaluation) The evaluation documents why I think it's OK to let the core themes inherit this template from core (particularly since it's an admin template in a non-admin theme) , but the changes that do occur are just enough to warrant getting additional opinions.
• system-status-counter.html.twig #14 (evaluation) Only necessary if it is decided that status-report-counter.html.twig (the above item) can't be inherited from core by Bartik/Umami

No work needed

• filter.caption.css #6 (evaluation)
• progress.module.css#7 (evaluation)
• system-modules-details.html.twig#8 (evaluation)
• system-modules-uninstall.html.twig#8 (evaluation)
• views-ui-views-listing-table.html.twig #9 (evaluation)
• install-page.html.twig#11 (evaluation)
• status-report-general-info.html.twig #13 (evaluation)

Need additional info

• status-report.html.twigWasn't sure how to get this to appear during manual testing - it looks like I may need to spoof an insufficient requirement but I figure it's more effective to see if another contributor can assist with this vs spending too much time in that rabbit hole.

Based on the above, the current patch updates the test with explanations for each skipped assets. For those definitely/potentially requiring additional work, the comment is to the right of the asset instead of above (intentionally breaking CS so they're easily spotted in PHPStorm. There's still a bit to do, but setting to Needs Review as a review would be good before proceeding with the next steps.

Discussed this with @lauriii and we determined that only one asset needs to be addressed: container-inline.module.css. Inheriting container-inline.module.css from core would result in unpleasant regressions in Umami and Bartik (see #10). This patch includes theme-specific regressions for those regressions. A followup for Umami was created - to see if there are benefits in refactoring to inherit from core: #3115710: Investigate changing container-inline rules in Umami. Bartik is not getting a followup as Olivero will be taking its place.

The other items I was unsure about in #16 were judged as not needing to be addressed:

• block.admin.css is admin styling and the only issues were occurring in non admin themes. In addition, it was determined that this approach should probably change, which is detailed in the followup: #3116003: UX issue with disabled blocks in block admin UI.
• status-report-counter.html.twig + system-status-counter.css: the changes are only present in non-admin themes, and do not "break" anything
• status-report.html.twig: we could not find any evidence that this is used by core themes

So this is now ready for a full review, this will cover the test and ensuring core themes can stop depending on Stable with no template or CSS issues.

#19.1 done.
#19.2 Right, my original plan was to address these independently - they were mentioned in #3110855: Plan for removing dependency to Stable in Bartik/Seven/Claro/Umami but a child issue wasn't yet created. Turns out it is much less complex than expected and actually in-scope here.
Both are files that were removed from core. The Stable versions would only be loaded via libraries-override: in Stable:, but since those files don't exist, there's nothing for Stable to replace.

file/drupal.file:
    css:
      theme:
        css/file.admin.css: css/file/file.admin.css

filter/drupal.filter:
    css:
      theme:
        css/filter.admin.css: css/filter/filter.admin.css

I updated the test with a comment that summarizes why this (and a template from the now-removed Simpletest) are skipped.

Good call on #21, I added comments to reflect this, and phrased it so it works with either commit status.

It's also a good cue to explicitly state that This issue should not be committed until #2821525: Update normalize.css to the most recent version is committed since it assumes that the fix provided by the normalize.css update is present.

You can see results of Wraith regression testing here: #3115223: Remove Stable as a base theme of core themes. This test includes the patch from #22, plus a patch from #3117217: Decouple core theme dependency on functions in stable.theme, Without all three of those being tested at once some regressions would be unavoidable, and it would be difficult to distinguish which ones were expected vs. which ones were a problem.

The results of the testing are encouraging, the only regression spotted was expected due to the Views list change detailed in #9 that we decided was acceptable.

#2821525: Update normalize.css to the most recent version landed so this is no longer postponed.

The test needed to be updated to account for normalize.css being added to Stable - something that happened earlier today.

Spotted a few nits in #30 moments after the testbot started.

Another nit.

Unrelated test failure, switching back to NR.
On the plus side, QuickeditIntegrationTest is taking some of the heat away from Media Library!