Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#23819 closed defect (bug) (fixed)

Twenty Thirteen: Handle activation on out of date WordPress installs

Reported by: obenland's profile obenland Owned by: lancewillett's profile lancewillett
Milestone: 3.6 Priority: high
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

Twenty Thirteen is meant to be not backwards compatible. Since we incorporate a lot of new functions and rely on markup changes that are introduced in 3.6, it really doesn't make sense to fail gracefully in a "slimmed down" version of the theme.

While we probably get some code in place on the .org side to prevent Twenty Thirteen from being downloaded through the WordPress admin, nothing prevents people from downloading it from the theme repository and upload it to their site through the zip installer.

We should be prepared to handle that in a better way than just failing with fatal errors.

Attachments (5)

23819.diff (1.4 KB) - added by kovshenin 11 years ago.
23819.1.diff (1.9 KB) - added by lancewillett 11 years ago.
23819.2.diff (1.1 KB) - added by lancewillett 11 years ago.
23819.2.2.diff (2.2 KB) - added by obenland 11 years ago.
23819.3.diff (2.8 KB) - added by obenland 11 years ago.

Download all attachments as: .zip

Change History (25)

#2 @lancewillett
11 years ago

  • Priority changed from normal to high

#3 @DrewAPicture
11 years ago

  • Cc xoodrew@… added

#4 follow-up: @kovshenin
11 years ago

I think this should be done on Core's side, the exact same way we deal with plugins. If a plugin triggers an error or produces unexpected output during activation - we deactivate it and show an error. We can do the same with themes and have Twenty Thirteen do version_compare and trigger_error.

#5 @WraithKenny
11 years ago

  • Cc Ken@… added

I've played around with the trigger_error() way of preventing activation of plugins. It seems weird to get a "Fatal Error" message when you are voluntarily trying to stop activation (not an error).

Now, because WP will output the text of the error message into the notification as HTML, you can include a <style> tag and do things to make it more user friendly, but perhaps this would be a nice time to give a better way to do this...

Perhaps a filter that can be set to true/string when a plugin/theme wants to display a "Activation Prevented" message. Core can check that filter when it checks for the errors no?

Last edited 11 years ago by WraithKenny (previous) (diff)

#6 in reply to: ↑ 4 @obenland
11 years ago

Replying to kovshenin:

I think this should be done on Core's side, the exact same way we deal with plugins.

I agree, having it done on core's side would be preferable. This won't help us with activations on previous versions however.

@kovshenin
11 years ago

#7 @kovshenin
11 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

23819.diff prevents theme activation if version is less than 3.6. Also note that 3.6-alpha-whatever is less than 3.6, so you'll want to change the "3.6" string to "3.6-alpha" to be able to activate the theme during development.

This, however, does not prevent theme previews.

#8 @obenland
11 years ago

I like the direction of your patch. Preventing theme previews is probably just a matter of another callback.

#9 @obenland
11 years ago

Talked about this ticked in dev chat - no need to rush it in before freeze, we can work on that in beta.

#11 @lancewillett
11 years ago

  • Keywords needs-testing removed

Tested in 3.5.1 and 3.4.2 and works nicely.

I'd prefer to keep it in a separate file, though -- for organization and keeping functions.php from getting too huge.

attachment:ticket:23819:23819.1.diff moves to an external include and removes esc_html() per SergeyBiryukov.

#12 @lancewillett
11 years ago

In 23816:

Twenty Thirteen: add back-compat function to avoid activation with older WordPress installs. Props kovshenin, see #23819 and #13780.

#13 follow-up: @lancewillett
11 years ago

@beaulebens reported that "3.6-alpha-23815" version failed the version_compare() check here.

#14 in reply to: ↑ 13 @kovshenin
11 years ago

Replying to lancewillett:

@beaulebens reported that "3.6-alpha-23815" version failed the version_compare() check here.

That's true, because 3.6-alpha-whatever is less than 3.6, same applies to beta :) Look at my note:

so you'll want to change the "3.6" string to "3.6-alpha" to be able to activate the theme during development.

I suggest either temporarily setting the string to "3.6-alpha" (which is less than 3.6 and 3.6-beta, but more than 3.5 with version_compare) or reverting the whole change until release.

Update: leaving it at 3.6-alpha would also work :)

Last edited 11 years ago by kovshenin (previous) (diff)

#15 @lancewillett
11 years ago

In 23825:

Twenty Thirteen: change version compare string to "3.6-alpha" to avoid an error for alpha and beta testers during development. See #23819, props kovshenin.

#16 @lancewillett
11 years ago

.2 prevent opening Theme Customizer.

@obenland
11 years ago

@obenland
11 years ago

#17 @lancewillett
11 years ago

.3 looks great -- nice commenting additions.

I might suggest "Twenty Thirteen is meant to be not backwards compatible" be changed to "Twenty Thirteen is not meant to be backwards compatible" to more clear.

#18 @obenland
11 years ago

You're the native! Feel free to tweak it to your linking.

#19 @lancewillett
11 years ago

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

In 23837:

Twenty Thirteen: better back compat handling by moving version compare before the include to avoid loading the file altogether. Also prevent Customizer views. Props obenland, closes #23819.

#20 @lancewillett
11 years ago

In 23838:

Twenty Thirteen: back compat file PHPDoc updates, thanks DrewAPicture for the nudge. See #23819.

Note: See TracTickets for help on using tickets.