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
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,432 pass(es). View
#1 broken-bartik.patch619 byteslauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,422 pass(es), 2 fail(s), and 0 exception(s). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

lauriii’s picture

Status: Active » Needs review
FileSize
619 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,422 pass(es), 2 fail(s), and 0 exception(s). View
davidhernandez’s picture

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

Status: Needs work » Needs review
FileSize
1.59 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,432 pass(es). View
886 bytes
davidhernandez’s picture

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

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

FileSize
451.13 KB
620.73 KB
463.91 KB
451.13 KB
620.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

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.