Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#27175 closed defect (bug) (fixed)

Profile Color Schemes Don't Change

Reported by: ipstenu's profile Ipstenu Owned by: nacin's profile 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 10 years ago.
27175.2.diff (1.0 KB) - added by SergeyBiryukov 10 years ago.
27175.3.diff (601 bytes) - added by Otto42 10 years ago.
27175.4.diff (1.7 KB) - added by SergeyBiryukov 10 years ago.
27175.5.diff (2.0 KB) - added by SergeyBiryukov 10 years ago.
27175.6.diff (1.2 KB) - added by SergeyBiryukov 10 years ago.

Download all attachments as: .zip

Change History (30)

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


10 years ago

#2 @SergeyBiryukov
10 years 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.

@nacin
10 years ago

#3 @nacin
10 years 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.

#4 @Ipstenu
10 years ago

  • Keywords has-patch added

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

#5 @nacin
10 years 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.

#6 @Otto42
10 years 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 10 years ago by Otto42 (previous) (diff)

#7 @SergeyBiryukov
10 years ago

In 27232:

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

see #27175.

@Otto42
10 years ago

#8 @Otto42
10 years 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.

#9 @SergeyBiryukov
10 years 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.

#10 @SergeyBiryukov
10 years ago

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

#11 @SergeyBiryukov
10 years ago

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

#12 follow-up: @SergeyBiryukov
10 years 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.

#13 in reply to: ↑ 12 ; follow-up: @SergeyBiryukov
10 years 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 :)

#14 in reply to: ↑ 13 @SergeyBiryukov
10 years 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 10 years ago by SergeyBiryukov (previous) (diff)

#15 @SergeyBiryukov
10 years ago

#27184 was marked as a duplicate.

#16 @nacin
10 years ago

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

#17 @nacin
10 years ago

#27186 was marked as a duplicate.

#18 follow-up: @bigjake36
10 years 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.

#19 in reply to: ↑ 18 @SergeyBiryukov
10 years 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?

#20 @bigjake36
10 years 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.

#21 follow-up: @Ipstenu
10 years 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 :)

#22 in reply to: ↑ 21 @bigjake36
10 years ago

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

#23 @GregRoss
10 years 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 :)

#24 @nacin
10 years 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.