Opened 12 years ago
Closed 10 years ago
#22030 closed defect (bug) (fixed)
After "clear background-color" in design options, still css is added to html-output
Reported by: | Ov3rfly | Owned by: | |
---|---|---|---|
Milestone: | 3.9 | Priority: | low |
Severity: | normal | Version: | 3.4 |
Component: | Customize | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
After you select "clear background-color" in design options, the default color still is added as inline css to html-output of frontend.
Tested with 3.4.2 and Twenty Twelve 1.0 theme
Steps to reproduce:
- Look at default html-output, no inline css for background-color
- Design -> Background -> Color, select e.g. color #123456, save
- Look at html-output, this is added:
<style type="text/css" id="custom-background-css"> body.custom-background { background-color: #123456; } </style>
- Design -> Background -> Color -> Click "clear" option, save
- Look at html-output, this is added with default value:
<style type="text/css" id="custom-background-css"> body.custom-background { background-color: #e6e6e6; } </style>
Expected behaviour: After "clear" option is used and default value #e6e6e6 (or only a #) is shown in backend, no inline css should be included in html output of frontend.
Attachments (7)
Change History (41)
#3
@
12 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
Yes, this is intentional with Twenty Twelve. WP now allows you to set a default background color in the theme functions file. When you "clear the color" in Appearance > Background it should revert back to the default color value.
#5
@
12 years ago
- Resolution wontfix deleted
- Status changed from closed to reopened
In a new wordpress install, where no value has been set for custom background-color via backend yet, no inline css is added to front end html, as expected.
After a value for custom background-color has been saved, the inline css appears, as expected.
But (and that's the bug here) it does not disappear again (as it should), if the value is "cleared" via the backend "clear" option.
The currently only way to remove that inline css again, is to delete the entry theme_mods_twentytwelve in database wp_options table.
Expected behaviour: No/empty/cleared setting = no extra inline css.
Note: The default background color comes from style.css, no inline code required. And about the theme functions.php file, if there is no add_theme_support() for custom-background, again no inline code required.
#13
follow-up:
↓ 16
@
11 years ago
Related code here has a lot of history. See [21054] and all of the tickets it links to. This does not regress any of those issues, correct?
#14
follow-up:
↓ 15
@
11 years ago
Can someone point me to this clear option? I can see only the "default color" button and the "remove background image" button. Not sure which is the "clear" one.
#16
in reply to:
↑ 13
@
11 years ago
Replying to nacin:
This does not regress any of those issues, correct?
It shouldn't and it hasn't in my testing. It removes the theme mod, so we just go back to the status after the initial theme activation.
#17
@
11 years ago
I mentioned on IRC that we would need the same for the Customizer.
My idea: Save the value, but don't print the inline CSS if it's the default color. That's the way like we do it in twentythirteen_header_style()
, twentytwelve_header_style()
and twentyeleven_header_style()
.
#18
follow-up:
↓ 19
@
11 years ago
The patch works good, the only thing is that it seems like specifying a color (even a default one) should result in the CSS output. If you CLEAR the color out and save, then I would expect the CSS to go away. I'd recommend changing the set_theme_mod('background_color', '');
to remove_theme_mod( 'background_color' );
but not checking for the default color first (just stick to the length checks).
#19
in reply to:
↑ 18
@
11 years ago
Replying to aaroncampbell:
The patch works good.
Yes, but not when you change it via the Customizer. See the linked IRC chat.
#21
@
11 years ago
I tried the following approach in _custom_background_cb()
to cover both the customizer and the background admin:
<?php $color = get_theme_mod( 'background_color' ); if ( get_theme_support( 'custom-background', 'default-color' ) == $color ) $color = false; if ( ! $background && ! $color ) return;
For some reason the customizer chokes on that and does not display the default color though. It also breaks themes that trust that the value in the theme mod gets used.
22030.1.2.diff fixes it for the background admin, but I'm really not sure (how) we can fix it for the customizer.
#22
@
11 years ago
- Keywords 3.7-early added
- Milestone changed from 3.6 to Future Release
- Version changed from 3.4.2 to 3.4
The customizer has some JS that mirrors the PHP equivalent. http://core.trac.wordpress.org/browser/trunk/wp-includes/js/customize-preview.js?rev=22798#L105
So the issue here is that we save the background color even when it is the default, rather than removing the mod?
Looks like get_background_color() would work just fine with this. Per the comment in _custom_background_cb() I added in [21002], the desired behavior does appear to be that we don't save the background color when it equals the default. // A default has to be specified in style.css. It will not be printed here.
We simply missed that in #20448.
I'm fine with this change, but let's do it early in a cycle because it could potentially trip up themes. And, let's figure out what needs to be done regarding the customizer.
#24
@
10 years ago
- Keywords commit added; 3.7-early removed
- Milestone changed from 3.7 to 3.8
Marking for commit for 3.8.
#27
@
10 years ago
- Keywords 3.9-early removed
- Milestone changed from Future Release to 3.9
I'm fine with this change, but let's do it early in a cycle because it could potentially trip up themes.
#28
@
10 years ago
- Keywords commit removed
With remove_theme_mod()
in .2.diff the two scenarios are now inconsistent. The legacy background screen removes the theme mod, while the Customizer saves an empty string.
22030.3.diff is an alternative approach which simply copies what we already do in the Twenties with the header text color.
#29
@
10 years ago
I tried something similar earlier, but the Customizer needs to be able to override the empty value (false
) from the theme_mod, with the the theme's default value, for the preview to work. Once we bail on the default color and don't output the CSS, the customizer has no chance to update that color.
I'll add a patch where it removes the theme_mod in the customizer too.
#30
@
10 years ago
22030.5.diff uses the check in _custom_background_cb()
and adds Customizer support for this approach.
This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.
10 years ago
#32
@
10 years ago
- Keywords commit added
22030.6.diff looks good to me, and seems to function as intended. Love the simplicity.
I believe this is intentional as #e6e6e6 is the default background color for Twenty Twelve. See #21226.
Suggest close.