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 18 months ago.
27175.2.diff (1.0 KB) - added by SergeyBiryukov 18 months ago.
27175.3.diff (601 bytes) - added by Otto42 18 months ago.
27175.4.diff (1.7 KB) - added by SergeyBiryukov 18 months ago.
27175.5.diff (2.0 KB) - added by SergeyBiryukov 18 months ago.
27175.6.diff (1.2 KB) - added by SergeyBiryukov 18 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 @ircbot18 months ago

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

comment:2 @SergeyBiryukov18 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.

@nacin18 months ago

comment:3 @nacin18 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 @Ipstenu18 months ago

  • Keywords has-patch added

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

comment:5 @nacin18 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 @Otto4218 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 18 months ago by Otto42 (previous) (diff)

comment:7 @SergeyBiryukov18 months ago

In 27232:

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

see #27175.

@SergeyBiryukov18 months ago

@Otto4218 months ago

comment:8 @Otto4218 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 @SergeyBiryukov18 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.

@SergeyBiryukov18 months ago

comment:10 @SergeyBiryukov18 months ago

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

comment:11 @SergeyBiryukov18 months ago

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

@SergeyBiryukov18 months ago

comment:12 follow-up: @SergeyBiryukov18 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.

@SergeyBiryukov18 months ago

comment:13 in reply to: ↑ 12 ; follow-up: @SergeyBiryukov18 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 @SergeyBiryukov18 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 18 months ago by SergeyBiryukov (previous) (diff)

comment:15 @SergeyBiryukov18 months ago

#27184 was marked as a duplicate.

comment:16 @nacin18 months ago

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

comment:17 @nacin18 months ago

#27186 was marked as a duplicate.

comment:18 follow-up: @bigjake3617 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 @SergeyBiryukov17 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 @bigjake3617 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: @Ipstenu17 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 @bigjake3617 months ago

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

comment:23 @GregRoss17 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 @nacin17 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.