WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 23 months ago

#20249 closed defect (bug) (fixed)

Remove custom header and background constants

Reported by: nacin Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

The custom header and custom background code are both weighed down heavily with constants and multiple functions for registering them.

For example, adding a custom header requires the simple add_custom_image_header() call, which takes three callback arguments. But, it also requires up to five constants to customize things. Additionally, for random default headers, you need to make an add_theme_support() call. If you don't want uploads, you need to do remove_theme_support('custom-header-uploads'). If you want the new flex-header stuff, they are additional arguments in add_theme_support(). If you want to remove support, you need to call remove_custom_image_header(), not remove_theme_support(). And, if you need to overload certain pieces in a child theme, good luck.

See, that's a lot going on. Let's simplify it.

A single add_theme_support('custom-header', $args) that handles everything except for default header registration. Multiple calls to it will result in the existing $args being merged with the newly passed args. So, for example, a child theme can remove uploads by simply calling add_theme_support('custom-header', array('uploads' => false) ).

Attached patch kills off the constants, removes extraneous functions, and causes everything to just wrap add_theme_support() and remove_theme_support().

It does some very particular things for backwards compatibility, like defining constants that aren't already defined, in case a plugin or parent theme references them, and to ensure that a child theme consistently overrides the wishes of the parent.

custom-header.php has not been ported over to rely on the new API. As a testament for this code's backwards compatibility (or potential thereof, in case of a minor bug or two), everything works.

I am going to try to crowd-source a test of all 90-something themes in http://wpcom-themes.svn.automattic.com to ensure full compatibility. (Some of the just-in-time stuff was a result of seeing that Twenty Eleven defined a header constant *after* add_custom_image_header(), which threw me in for a loop.)

Attachments (3)

20249.diff (18.9 KB) - added by nacin 2 years ago.
20249.2.diff (35.7 KB) - added by nacin 2 years ago.
theme.php.patch (579 bytes) - added by azaozz 2 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 nacin2 years ago

This is a follow-up from #17242.

comment:2 nacin2 years ago

I just deleted my local patch accidentally. There is death-to-constants-and-lame-functions.diff:ticket:17242, which I will refresh shortly with a few more changes I had made.

nacin2 years ago

comment:3 nacin2 years ago

  • Keywords has-patch added

comment:4 toscho2 years ago

  • Cc info@… added

nacin2 years ago

comment:5 nacin2 years ago

20249.2.diff tears up custom-header.php as well. No more background or header constants are referenced outside of the back compat code. I'll post a summary of everything the patch does API-wise, as well as testing notes, over the weekend.

comment:6 nacin2 years ago

In [20212]:

Introduce new registration methods for custom headers and custom backgrounds. Backwards compatible, but old methods will be deprecated. see #20249. see #17242.

Custom header: Use add_theme_support('custom-header', $args) instead of add_custom_image_header(). Deprecates all use of constants.

  • HEADER_TEXTCOLOR is now (string) 'default-text-color'.
  • NO_HEADER_TEXT is nowi ! (bool) 'header-text'.
  • HEADER_IMAGE_WIDTH (and _HEIGHT) are now (int) 'width' and 'height'.
  • HEADER_IMAGE is now (string) 'default-image'.
  • The 3.4 arguments 'suggested-width' and 'suggested-height' are now just 'width' and 'height' (they are "suggested" when flex-width and flex-height are set).
  • Callback arguments for add_custom_image_header() can now be passed to add_theme_support().

Custom background: Use add_theme_support('custom-background, $args) instead of add_custom_background(). Deprecates all use of constants.

  • BACKGROUND_COLOR is now (string) 'default-color'.
  • BACKGROUND_IMAGE is now (string) 'default-image'.
  • Callback arguments for add_custom_background() can now be passed to add_theme_support().

Inheritance: add_theme_support() arguments for custom headers and custom backgrounds is a first-one-wins situation. This is not an unusual paradigm for theming as a child theme (which is included first) overrides a parent theme.

  • Once an argument is explicitly set, it cannot be overridden. You must hook in earlier and set it first.
  • Any argument that is not explicitly set before WP is loaded will inherit the default value for that argument.
  • It is therefore possible for a child theme to pass minimal arguments as long as the parent theme specifies others that may be necessary.
  • Allows for a child theme to alter callbacks for <head> and preview (previously, calling add_custom_image_header more than once broke things).
  • The just-in-time bits ensure that arguments fall back to default values, that the values of all constants are considered (such as one defined after an old add_custom_image_header call), and that all constants are defined (so as to be backwards compatible).

get_theme_support(): Introduce new second argument, which headers and backgrounds leverage to return an argument. current_theme_supports() already supported checking the truthiness of the argument.

  • For example, get_theme_support( 'custom-header', 'width' ) will return the width specified during registration.
  • If you had wanted the default image, use get_theme_support( 'custom-header', 'default-image' ) instead of HEADER_IMAGE.

Deprecate remove_custom_image_header(), remove_custom_background(). Use remove_theme_support('custom-header'), 'custom-background'.

Deprecate short-lived custom-header-uploads internal support; this is now (bool) 'uploads' for add_theme_support().

New 3.4 functions renamed or removed: Rename get_current_header_data() to get_custom_header(). Remove get_header_image_width() and _height() in favor of get_custom_header()->width and height.

comment:7 nacin2 years ago

In [20213]:

Fix copy-paste issue. see #20249.

comment:8 scribu2 years ago

Related: #18887

comment:9 nacin2 years ago

In [20214]:

The word 'support' does not have a overly long 'p'. see #20249.

comment:10 chipbennett2 years ago

  • Cc chip@… added

comment:11 nacin2 years ago

In [20218]:

Deprecate add_custom_image_header(), remove_custom_image_header(), add_custom_background(), remove_custom_background(). Replacements are add_theme_support() and remove_theme_support(). see #20249.

comment:12 scribu2 years ago

Nitpick:

_deprecated_function( __FUNCTION__, '3.4', 'add_theme_support(\'custom-header\', $args)' );

I thought the policy was to avoid backslashes if possible.

Last edited 2 years ago by scribu (previous) (diff)

comment:13 scribu2 years ago

Ah, if you used double-quotes, you'd have to add a backslash anyway: \$args. Nevermind.

comment:14 nacin2 years ago

In [20220]:

Don't suggest only add_theme_support('custom-background') -- suggest it with $args even if it wasn't called with any arguments. In particular, default-color should be used by themes as a good user experience improvement. see #20249.

comment:15 follow-up: iandstewart2 years ago

I find the callbacks, 'callback' and 'admin-header-callback', to be somewhat confusingly named. Of course, that could just be me. :) 'head-callback' and 'admin-head-callback' might be more helpful.

comment:16 nacin2 years ago

In [20221]:

In new add_theme_support() logic, fix logic inversion, define HEADER_IMAGE in a simpler way, and force args to be an array. see #20249.

comment:17 nacin2 years ago

In [20222]:

Update twentyeleven/functions.php to use the new add_theme_support() calls for backgrounds and headers. props sabreuse for initial patch. see #20265. see #20249.

comment:18 nacin2 years ago

In [20225]:

Move Twenty Ten and the rest of Twenty Eleven to add_theme_support() for headers and backgrounds. props sabreuse for initial patch. fixes #20265. see #20249.

comment:19 in reply to: ↑ 15 ; follow-up: nacin2 years ago

Replying to iandstewart:

I find the callbacks, 'callback' and 'admin-header-callback', to be somewhat confusingly named. Of course, that could just be me. :) 'head-callback' and 'admin-head-callback' might be more helpful.

I agree. I called it 'callback' because we were already using that internally, but we could scrap that as it wasn't behavior anyone else would use. (It was only used to remove the hook in remove_custom_image_header().)

If I had to pick three names, it would be:

  • wp_head
  • admin_head
  • admin-preview-callback

Obviously, those are not standardized. I could go for:

  • wp-head-callback
  • admin-head-callback
  • admin-preview-callback

Essentially, dropping "header", as it's the admin <head>, not the header, and cleaning up the unsemantic admin-image-div-callback name.

comment:20 nacin2 years ago

I do like the simplicity of 'callback', though, especially since it is frequently going to be the only callback used (for custom headers, at least; custom background callbacks are not often used).

comment:21 in reply to: ↑ 19 toscho2 years ago

Replying to nacin:

Obviously, those are not standardized. I could go for:

  • wp-head-callback
  • admin-head-callback
  • admin-preview-callback

Sounds good. Easy to understand and to remember.

comment:22 follow-up: Jayjdk2 years ago

I can't get my default background image to show.

Code used:

add_theme_support( 'custom-background', 'default-color' => 'e3ecf2', 'default-image' => '%s/images/bg.png' ) );

By changing line 204 in wp-admin/custom-background.php from

$background_styles .= ' background-image: url(\'' . get_theme_mod('background_image_thumb', '') . '\');'	

to

$background_styles .= ' background-image: url(\'' . get_background_image() . '\');'

it will show. I don't know if it's the best solution but it works.

comment:23 in reply to: ↑ 22 nacin2 years ago

Replying to Jayjdk:

I can't get my default background image to show.

Code used:

add_theme_support( 'custom-background', 'default-color' => 'e3ecf2', 'default-image' => '%s/images/bg.png' ) );

By changing line 204 in wp-admin/custom-background.php from

$background_styles .= ' background-image: url(\'' . get_theme_mod('background_image_thumb', '') . '\');'	

to

$background_styles .= ' background-image: url(\'' . get_background_image() . '\');'

it will show. I don't know if it's the best solution but it works.

Did this work with constants under 3.3? Doesn't look like it would.

comment:24 follow-up: Jayjdk2 years ago

Nope. It doesn't work in 3.3 with constants

comment:25 nacin2 years ago

[20231] Callbacks for custom headers and custom backgrounds registered through add_theme_support() are now wp-head-callback, admin-head-callback, and admin-preview-callback.

comment:26 in reply to: ↑ 24 nacin2 years ago

Replying to Jayjdk:

Nope. It doesn't work in 3.3 with constants

The default color and default image should both be defined in the stylesheet, which is why the _custom_background_cb() does not handle them. The reason to additionally define them in PHP is so the Background and Header screens can provide a better experience as they relate to WYSIWYG and resetting to defaults. (That said, both screens are pretty bad at this, but they can be made better.)

comment:27 follow-up: Jayjdk2 years ago

But shouldn't the background preview show the defined background image? It shows the defined background color

comment:28 in reply to: ↑ 27 nacin2 years ago

Replying to Jayjdk:

But shouldn't the background preview show the defined background image? It shows the defined background color

Yes. At the moment, the admin screens poorly handle default images and colors. And sorry, I didn't realize your code in comment 22 came from wp-admin/custom-background.php; I figured it came from the front-end _custom_background_cb(). We are on the same page.

comment:29 azaozz2 years ago

Seems [20231] will need a check whether a callback exists for add_action( 'wp_head', $args[0]['wp-head-callback'] ); on line 1444 in theme.php same as on line 1430. Getting an empty action when using older theme (not Twenty Ten or Eleven)

Last edited 2 years ago by azaozz (previous) (diff)

azaozz2 years ago

comment:30 nacin2 years ago

What is the theme using to register a custom background?

If a custom background's callback is empty, it uses _custom_background_cb(). This is different from headers, which allows no callback. Hence why there is no empty check on 1444 but there is one on 1430.

comment:31 nacin2 years ago

In [20246]:

Only pass arguments from the deprecated add_custom_background() or add_custom_image_header() to add_theme_support() if the argument was actually supplied. With [20212], default argument values (like wp-head-callback=_custom_background_cb) only work for arguments that were not explicitly set, so don't set empty values. see #20249.

comment:32 nacin2 years ago

[20246] fixes the issue reported by azaozz.

comment:33 ryan2 years ago

In [20265]:

s/background/header/ in add_custom_image_header() deprecated function. see #20249

comment:34 nacin2 years ago

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

New tickets for any bug reports or enhancements.

comment:35 nacin23 months ago

In [20901]:

Preview by default the registered default image for custom backgrounds. props mfields, billerickson.

If there is a default color registered, show a 'Default' action rather than a 'Clear' action, as clearing the value would simply return to the default.

Make current_theme_supports() accept a second argument for 'custom-background' requests, the same as get_theme_support(). Missed in earlier changes, see #20249.

fixes #20734, fixes #18041.

Note: See TracTickets for help on using tickets.