Follow-up to #2350823: Use the Classy theme in the Testing profile

Problem/Motivation

Following the test profile changes to switch the default theme to Classy, we introduced a major regression. The layout.css file causes a sub-themes layout.css file that uses the same path not to load. This has broken Bartik!

Proposed resolution

Fix! If not quickly, roll back bbb4de91bc90ba016b82acd908cc0edd71830bf9.

Remaining tasks

We may want to follow up and do something about the test that checks for the layout.css file, because I don't like the idea of Classy trying to load it if it is empty.

CommentFileSizeAuthor
#7 after-patch-2.png620.73 KBemma.maria
#7 after-patch-1.png451.13 KBemma.maria
#7 before-patch.png463.91 KBemma.maria
#7 after-patch-2.png620.73 KBemma.maria
#7 after-patch-1.png451.13 KBemma.maria
#4 interdiff.txt886 byteslauriii
#4 broken-bartik-2380605-4.patch1.59 KBlauriii
#1 broken-bartik.patch619 byteslauriii
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Active » Needs review
FileSize
broken-bartik.patch619 bytes
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented

A little more info. The layout file in Classy was added in classy.info. Bartik uses a library for its base, so the info file took precedence. It does not appear to have broken Seven, because Seven adds its layout file in the info file directly and not through a library.

Status: Needs review » Needs work

The last submitted patch, 1: broken-bartik.patch, failed testing.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Needs work » Needs review
FileSize
broken-bartik-2380605-4.patch1.59 KB
interdiff.txt886 bytes
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented
Category: Task » Bug report

Just changing to bug.

I think the larger problem is also a bug. Why can a base them override a subtheme?

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC, this is a good move, we want to get rid of CSS in .info files anyway :).

And then this bug goes away automatically.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
FileSize
after-patch-1.png451.13 KB
after-patch-2.png620.73 KB
before-patch.png463.91 KB
after-patch-1.png451.13 KB
after-patch-2.png620.73 KB

Bartik currently has a broken layout right now. In the source you can see only Classy's layout.css being loaded and not Bartik's layout.css.

With the patch applied Bartik looks like it should again. And you can see in the source and when you inspect that Bartik's layout.css file is being loaded again and in use.

Therefore this patch fixes what is currently broken at head.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Given #2377397: Themes should use libraries, not individual stylesheets I think we should move ahead here without adding specific tests. This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed bdafa44 and pushed to 8.0.x. Thanks!

  • alexpott committed bdafa44 on 8.0.x 
    Issue #2380605 by emma.maria, lauriii: Bartik layout broken

Status: Fixed » Closed (fixed)

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