WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#23819 closed defect (bug) (fixed)

Twenty Thirteen: Handle activation on out of date WordPress installs

Reported by: obenland Owned by: lancewillett
Priority: high Milestone: 3.6
Component: Bundled Theme Version: trunk
Severity: normal Keywords: has-patch
Cc: xoodrew@…, Ken@…

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 3 months ago.
23819.1.diff (1.9 KB) - added by lancewillett 3 months ago.
23819.2.diff (1.1 KB) - added by lancewillett 3 months ago.
23819.2.2.diff (2.2 KB) - added by obenland 3 months ago.
23819.3.diff (2.8 KB) - added by obenland 3 months ago.

Download all attachments as: .zip

Change History (25)

comment:2 lancewillett3 months ago

  • Priority changed from normal to high

comment:3 DrewAPicture3 months ago

  • Cc xoodrew@… added

comment:4 follow-up: kovshenin3 months 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.

comment:5 WraithKenny3 months 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 3 months ago by WraithKenny (previous) (diff)

comment:6 in reply to: ↑ 4 obenland3 months 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.

kovshenin3 months ago

comment:7 kovshenin3 months 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.

comment:8 obenland3 months ago

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

comment:9 obenland3 months ago

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

lancewillett3 months ago

comment:11 lancewillett3 months 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.

comment:12 lancewillett3 months ago

In 23816:

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

comment:13 follow-up: lancewillett3 months ago

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

comment:14 in reply to: ↑ 13 kovshenin3 months 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 3 months ago by kovshenin (previous) (diff)

comment:15 lancewillett3 months 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.

lancewillett3 months ago

comment:16 lancewillett3 months ago

.2 prevent opening Theme Customizer.

obenland3 months ago

obenland3 months ago

comment:17 lancewillett3 months 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.

comment:18 obenland3 months ago

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

comment:19 lancewillett3 months 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.

comment:20 lancewillett3 months 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.