WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#23819 closed defect (bug) (fixed)

Twenty Thirteen: Handle activation on out of date WordPress installs

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

Download all attachments as: .zip

Change History (25)

comment:2 lancewillett13 months ago

  • Priority changed from normal to high

comment:3 DrewAPicture13 months ago

  • Cc xoodrew@… added

comment:4 follow-up: kovshenin13 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 WraithKenny13 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 <script> 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?

Version 0, edited 13 months ago by WraithKenny (next)

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

kovshenin13 months ago

comment:7 kovshenin13 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 obenland13 months ago

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

comment:9 obenland13 months ago

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

lancewillett13 months ago

comment:11 lancewillett13 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 lancewillett13 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: lancewillett13 months ago

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

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

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

lancewillett13 months ago

comment:16 lancewillett13 months ago

.2 prevent opening Theme Customizer.

obenland13 months ago

obenland13 months ago

comment:17 lancewillett13 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 obenland13 months ago

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

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