WordPress.org

Make WordPress Core

#27175 closed defect (bug) (fixed)

Profile Color Schemes Don't Change

Reported by: Ipstenu Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.9
Component: Script Loader Keywords: has-patch
Focuses: administration Cc:

Description

On trunk the color schemes stopped working.

Go to your profile, pick a style. The temp display works (yay).

Click save and it reverts to default, but the style you picked is still selected.

http://twitpic.com/dw8hlf (@briangardner has the same ish)

admin-color-ectoplasm is still showing in my body class but I don't see wp-admin/css/colors/ectoplasm/colors.min.css in my resources, so it's just not getting called.

Possibly related to #26669 ?

Attachments (6)

27175.diff (693 bytes) - added by nacin 13 months ago.
27175.2.diff (1.0 KB) - added by SergeyBiryukov 13 months ago.
27175.3.diff (601 bytes) - added by Otto42 13 months ago.
27175.4.diff (1.7 KB) - added by SergeyBiryukov 13 months ago.
27175.5.diff (2.0 KB) - added by SergeyBiryukov 13 months ago.
27175.6.diff (1.2 KB) - added by SergeyBiryukov 13 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 @ircbot13 months ago

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

comment:2 @SergeyBiryukov13 months ago

  • Component changed from Appearance to Script Loader
  • Milestone changed from Awaiting Review to 3.9

Could not reproduce locally, but reproduced on make/core. Moving for investigation.

@nacin13 months ago

comment:3 @nacin13 months ago

It's because something is initializing WP_Styles on or before init, versus waiting until admin_enqueue_scripts. Color schemes are registered on admin_init.

27175.diff works for me.

comment:4 @Ipstenu13 months ago

  • Keywords has-patch added

Confirmed, nacin's patch works for me too :)

comment:5 @nacin13 months ago

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

In 27228:

Make sure color schemes are registered when WP_Styles is initialized early.

fixes #27175. see #26669.

comment:6 @Otto4213 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This change breaks calls to load-styles.php resulting in broken admin screens.

Both did_action() and has_action() are not defined in load-styles.php.

Edit: remove_action() is also not defined.

Last edited 13 months ago by Otto42 (previous) (diff)

comment:7 @SergeyBiryukov13 months ago

In 27232:

Revert [27228], as it doesn't work with SCRIPT_DEBUG off.

see #27175.

@SergeyBiryukov13 months ago

@Otto4213 months ago

comment:8 @Otto4213 months ago

27175.3.diff works for me to fix the problem on my sandbox, although it's not without its flaws. Perhaps combined with portions of Sergey's patch (the is_admin() check), this would be a feasible alternative.

comment:9 @SergeyBiryukov13 months ago

I was trying something like 27175.3.diff too, but it throws a fatal error if a plugin calls wp_register_script() before init:

Fatal error: Call to undefined function is_rtl() in wp-includes/general-template.php on line 2102

27175.2.diff fixes color schemes if a plugin registers a script before admin_init (e.g. on init), but still doesn't work if it does that before init.

Ideally, color schemes should work even if a plugin registers a script before init.

@SergeyBiryukov13 months ago

comment:10 @SergeyBiryukov13 months ago

27175.4.diff seems to work in any scenario I can think of.

comment:11 @SergeyBiryukov13 months ago

Oh, it removes the ability to de-register the default schemes. I guess we want to keep it.

@SergeyBiryukov13 months ago

comment:12 follow-up: @SergeyBiryukov13 months ago

27175.5.diff brings back some of [27232]. Works in any scenario I can think of and keeps the ability to de-register the default schemes.

The did_action( 'admin_init' ) check in [27232] appears to be redundant, as wp_default_styles() always runs earlier.

@SergeyBiryukov13 months ago

comment:13 in reply to: ↑ 12 ; follow-up: @SergeyBiryukov13 months ago

Replying to SergeyBiryukov:

The did_action( 'admin_init' ) check in [27232] appears to be redundant, as wp_default_styles() always runs earlier.

This leads to 27175.6.diff :)

comment:14 in reply to: ↑ 13 @SergeyBiryukov13 months ago

Replying to SergeyBiryukov:

The did_action( 'admin_init' ) check in [27232] appears to be redundant, as wp_default_styles() always runs earlier.

Well, maybe not. I got that impression looking at http://codex.wordpress.org/Plugin_API/Action_Reference.

It turned out that without plugins registering scripts too early, wp_default_styles() actually does not run earlier than admin_init in the admin.

Still, 27175.6.diff makes the most sense to me here.

Last edited 13 months ago by SergeyBiryukov (previous) (diff)

comment:15 @SergeyBiryukov13 months ago

#27184 was marked as a duplicate.

comment:16 @nacin13 months ago

Still thinking about this. Maybe we should just revert [27203].

comment:17 @nacin13 months ago

#27186 was marked as a duplicate.

comment:18 follow-up: @bigjake3613 months ago

I made the changes, but not all color schemes work when you do to save them. The basic ones seem to work, but the creative color schemes from the plugins will not save. Also, as posted, once the files have been updated, the new alpha updates over write the corrections and it's back to square one.

comment:19 in reply to: ↑ 18 @SergeyBiryukov13 months ago

Replying to bigjake36:

I made the changes, but not all color schemes work when you do to save them. The basic ones seem to work, but the creative color schemes from the plugins will not save.

Could you give an example of a plugin color schemes of which cannot be saved?

comment:20 @bigjake3613 months ago

It doesn't matter what colors I select, it will not save to profile unless I edit the patches in. Once I do that it does work to some extent until the next Alpha update. The update seems to replace the edited files and I'm back to square one. I then go back in and edit the files and it works with the basic colors. The plug in however "Admin Color Scheme" doesn't work at all other than it shows the colors, you can select the colors, but it will not save the colors.

comment:21 follow-up: @Ipstenu13 months ago

What Jake said. No color scheme, except default, is retained after selection and saving. Example? I want Blue. I click the option, my profile shows me Blue so I can verify that's the color, I hit save. Boom. Back to default.

As @nacin said, though, they know the fix, just have to design which way to jump :)

comment:22 in reply to: ↑ 21 @bigjake3613 months ago

Are there any updates to this issue or perminate fix's yet ??

comment:23 @GregRoss13 months ago

I've been doing some poking at this since 3.9b1 hit yesterday as my plugin uses custom color schemes. I found the issue is that the previous recommendation for plugins was to use the 'admin_init' action to hook the calls for wp_admin_css_color(), but 'admin_init' doesn't fire until after script-loader.php runs so script-loader can't find the right css files and drops back to the default color scheme.

Replacing 'admin_init' with just 'init' in my plugin "fixes" the problem, but of course runs on EVERY page load, not just the admin page loads.

I haven't found another action yet that runs before the script-loader and only on admin pages. Probably a few different possible solutions:

  • create a new action for admin color schemes, something like 'admin_colors' (or find one that fits the requirement). Downside is that all plugins that add color schemes have to be updated.
  • move the loading of the admin color css files until after admin_init.
  • revert back to the old code which blindly trusted that the css files would be there when need :)

comment:24 @nacin13 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 27515:

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

fixes #27175. see #20729.

Note: See TracTickets for help on using tickets.