Make WordPress Core

Opened 14 years ago

Closed 13 years ago

#14837 closed enhancement (fixed)

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

Reported by: jorbin's profile jorbin Owned by: jorbin's profile 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 14 years ago.
custom-header-15606.diff (2.1 KB) - added by jorbin 14 years ago.
custom-header-15606.2.diff (2.1 KB) - added by jorbin 14 years ago.
custom-header-15606.3.diff (2.0 KB) - added by jorbin 14 years ago.
Switch strings

Download all attachments as: .zip

Change History (18)

#1 follow-up: @ericmann
14 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.

#2 in reply to: ↑ 1 ; follow-up: @nacin
14 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.

#3 in reply to: ↑ 2 ; follow-up: @jorbin
14 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.

#4 follow-up: @hakre
14 years ago

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

#5 in reply to: ↑ 4 @jorbin
14 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.

#6 @nacin
14 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.

#7 @nacin
14 years ago

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

#8 @jorbin
14 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.

#9 @jorbin
14 years ago

  • Version 3.0 deleted

@jorbin
14 years ago

Switch strings

#10 in reply to: ↑ 3 @kahless
14 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.

#11 @nacin
14 years ago

Also consider consistent implementation with #14903.

#12 @nacin
13 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.

#13 @Denis-de-Bernardy
13 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?

#14 @Denis-de-Bernardy
13 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.