WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#14837 closed enhancement (fixed)

Themes should allow custom headers to be selectable, but not uploadable

Reported by: jorbin Owned by: jorbin
Milestone: 3.1 Priority: normal
Severity: minor Version:
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

Currently themes that allow custom headers have to allow for both the selection of default headers and the ability for themes to upload there own. This patch adds the ability for themes to not allow uploaded custom headers.

Attachments (4)

custom-header-15605.diff (1.7 KB) - added by jorbin 4 years ago.
custom-header-15606.diff (2.1 KB) - added by jorbin 4 years ago.
custom-header-15606.2.diff (2.1 KB) - added by jorbin 4 years ago.
custom-header-15606.3.diff (2.0 KB) - added by jorbin 4 years ago.
Switch strings

Download all attachments as: .zip

Change History (18)

jorbin4 years ago

comment:1 follow-up: ericmann4 years ago

Honestly, if a theme doesn't want to support the uploading of custom headers, that's up to the theme ... why does this patch need to be in core? I don't see the point.

comment:2 in reply to: ↑ 1 ; follow-up: nacin4 years ago

Possible better way of implementing this: We should instead register theme support for "custom-header-uploads" by default, and allow the theme to remove said theme support as a way to enable this.

Replying to ericmann:

Honestly, if a theme doesn't want to support the uploading of custom headers, that's up to the theme ... why does this patch need to be in core? I don't see the point.

Because the theme would still want the ability for registered headers to be chosen. Currently the custom-header UI won't support that.

I see use cases here for sure. Not a good idea for a distributed theme, but rather in instances where a user on a site might be allowed to edit_theme_options but should not be allowed to modify anything beyond what the admins have specifically set up for the site.

comment:3 in reply to: ↑ 2 ; follow-up: jorbin4 years ago

Replying to nacin:

Possible better way of implementing this: We should instead register theme support for "custom-header-uploads" by default, and allow the theme to remove said theme support as a way to enable this.

That makes sense. I'll update the patch accordingly.

Replying to ericmann:

Honestly, if a theme doesn't want to support the uploading of custom headers, that's up to the theme ... why does this patch need to be in core? I don't see the point.

Exactly, it should be up to the theme. Right now it's not even an option. Custom headers are all or nothing.

Because the theme would still want the ability for registered headers to be chosen. Currently the custom-header UI won't support that.

I see use cases here for sure. Not a good idea for a distributed theme, but rather in instances where a user on a site might be allowed to edit_theme_options but should not be allowed to modify anything beyond what the admins have specifically set up for the site.

Another use case I see is being on Multi Site networks that want to offer a range of branded headers, but not wanting to give the user the option to upload an unbranded one.

comment:4 follow-up: hakre4 years ago

What for themes that want to offer upload but no selection of registered headers?

comment:5 in reply to: ↑ 4 jorbin4 years ago

Replying to hakre:

What for themes that want to offer upload but no selection of registered headers?

I believe They just have to not register any default headers.

jorbin4 years ago

comment:6 nacin4 years ago

  • Milestone changed from Awaiting Review to 3.1

Only downside to this is that remove_theme_support('custom-header') noops. So it is potentially confusing that you need to use add_custom_image_header() and such, but then remove_theme_support('custom-header-uploads'). That said I really like the simplicity of it.

comment:7 nacin4 years ago

Also, instead of the new string for steps 2/3, we can probably just use "Cheatin, eh?"

comment:8 jorbin4 years ago

  • Owner set to jorbin
  • Status changed from new to accepted
  • Version set to 3.0

Since it is removing support for a feature, I think it makes sense and is really the most simple way. The only true confusing part is that it must be used after add_custom_image_header is called, but I think that can be solved through education.

comment:9 jorbin4 years ago

  • Version 3.0 deleted

jorbin4 years ago

Switch strings

comment:10 in reply to: ↑ 3 kahless4 years ago

Replying to jorbin:

Replying to nacin:

Possible better way of implementing this: We should instead register theme support for "custom-header-uploads" by default, and allow the theme to remove said theme support as a way to enable this.

That makes sense. I'll update the patch accordingly.

Replying to ericmann:

Honestly, if a theme doesn't want to support the uploading of custom headers, that's up to the theme ... why does this patch need to be in core? I don't see the point.

Exactly, it should be up to the theme. Right now it's not even an option. Custom headers are all or nothing.

Because the theme would still want the ability for registered headers to be chosen. Currently the custom-header UI won't support that.

I see use cases here for sure. Not a good idea for a distributed theme, but rather in instances where a user on a site might be allowed to edit_theme_options but should not be allowed to modify anything beyond what the admins have specifically set up for the site.

Another use case I see is being on Multi Site networks that want to offer a range of branded headers, but not wanting to give the user the option to upload an unbranded one.

This is exactly the situation in which I find myself. I am designing a faculty research showcase and one of the things we want is to allow each faculty member to be able to customize their research site's look to some degree, but remain within the branding and marketing of the college. We will be in a multisite setting where the main site will have a custom theme and I'd like to use Twenty Ten as the theme on each faculty research site where they can choose from the header images we provide.

comment:11 nacin4 years ago

Also consider consistent implementation with #14903.

comment:12 nacin4 years ago

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

(In [15828]) Allow disabling of custom header uploads via remove_theme_support. props jorbin, fixes #14837.

comment:13 Denis-de-Bernardy4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening, because I believe this introduces backwards compatibility issues.

Shouldn't we automatically support header uploads when the theme supports custom headers, and then allow theme authors to explicitly disable header uploads using remove_theme_supports() or whatever the function's name is?

Alternatively, would it not make sense to simply add a new capability?

comment:14 Denis-de-Bernardy4 years ago

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

My bad, I misread the patch...

Note: See TracTickets for help on using tickets.