WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 months ago

Last modified 6 weeks ago

#20729 closed defect (bug) (wontfix)

Improve Admin Colors CSS enqueuing

Reported by: westi Owned by: nacin
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Script Loader Keywords: needs-patch westi-likes 3.5-early
Focuses: administration Cc:

Description (last modified by westi)

The hacky way in which we enqueue the admin colors css means that you get different ordering of the CSS between the development version of the scripts and the script-loader sourced version of the scripts.

This can cause CSS bugs which don't show when developing but do when running with the script loader - e.g. #16827

It also makes it harder to implement a custom css concatenator.

We should do something like - http://core.trac.wordpress.org/attachment/ticket/16827/colors-hacked-fixed.diff

To make this properly enqueue the styles.

We should also make this kind of call fire a _doing_it_wrong() as true is not a url :)

$styles->add( 'colors', true, array('wp-admin') );

This code harks back from [7976]

Change History (10)

comment:1 westi2 years ago

  • Description modified (diff)
  • Keywords changed from needs-patch westi-likes, 3.5-early to needs-patch westi-likes 3.5-early

comment:2 ocean903 months ago

  • Component changed from Appearance to Administration

comment:3 helen3 months ago

  • Milestone changed from Future Release to 3.9

Moving to 3.9 to investigate along with the colors.css merge (#18380). Can anybody enlighten me as to why this was done this way? For the login screen?

comment:4 ircbot2 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:5 nacin2 months ago

In 27111:

Fix the conditional enqueueing/printing of colors stylesheets, without breaking dependencies.

fixes #18380.
see #20729 which should properly fix this.

comment:6 nacin2 months ago

  • Component changed from Administration to Script Loader
  • Focuses administration added

[18577/trunk/wp-admin/admin-header.php] changed things a bit — wp_admin_css() is no longer used.

I've been tracing through this and I don't think we can make any changes that would look any less hacky.

We no longer always print colors (as of [27111] / #18380). Since it is to override other things, realistically it should go after all core styles that have the potential to be lumped into load-styles.css — that is anything other than ie.css, really. The way we handle colors isn't the problem. Our dependencies code doesn't guarantee order. But neither does load-scripts/styles, and certainly without the expectation of it being consistent between the two. Even regular CSS files could appear in an order different from what is expected.

If we ever had an independent CSS file again that needs to load *after* colors (which is entirely possible), it should declare colors as a dependency. But even that would not have worked because of how our dependencies code was not originally written for concatenation, which was shoehorned in later.

Very simply there's no good way to fix this, but where we are at now (thanks to changes in 3.8 and now 3.9 to how color schemes work) means the breakage is much smaller.

I think it would be nice to make it so the handle gets its source up front, for the benefit of a custom concatenator. Otherwise this is a wontfix.

comment:7 nacin2 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27203:

Simplify how admin color schemes are enqueued.

Removes a piece of [27111].

fixes #20729.

comment:8 nacin2 months ago

In 27204:

Remove accidental change in [27203]. see #20729.

comment:9 nacin6 weeks ago

In 27515:

Revert [27203], restore JIT color scheme stylesheets. Restores [27111].

fixes #27175. see #20729.

comment:10 nacin6 weeks ago

  • Milestone 3.9 deleted
  • Resolution changed from fixed to wontfix

Per #27175.

Note: See TracTickets for help on using tickets.