#23819 closed defect (bug) (fixed)
Twenty Thirteen: Handle activation on out of date WordPress installs
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (25)
comment:1
SergeyBiryukov
— 3 months ago
comment:2
lancewillett
— 3 months ago
- Priority changed from normal to high
comment:3
DrewAPicture
— 3 months ago
- Cc xoodrew@… added
comment:4
follow-up:
↓ 6
kovshenin
— 3 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
WraithKenny
— 3 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?
comment:6
in reply to:
↑ 4
obenland
— 3 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.
comment:7
kovshenin
— 3 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
obenland
— 3 months ago
I like the direction of your patch. Preventing theme previews is probably just a matter of another callback.
comment:9
obenland
— 3 months ago
Talked about this ticked in dev chat - no need to rush it in before freeze, we can work on that in beta.
comment:10
SergeyBiryukov
— 3 months ago
23819.diff looks good to me.
esc_html() is probably not necessary, we don't use it in other messages hooked to admin_notices:
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-admin/includes/ms.php#L270
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-admin/includes/update.php#L118
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-admin/includes/user.php#L352
lancewillett
— 3 months ago
comment:11
lancewillett
— 3 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
lancewillett
— 3 months ago
In 23816:
comment:13
follow-up:
↓ 14
lancewillett
— 3 months ago
@beaulebens reported that "3.6-alpha-23815" version failed the version_compare() check here.
comment:14
in reply to:
↑ 13
kovshenin
— 3 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 :)
comment:15
lancewillett
— 3 months ago
In 23825:
lancewillett
— 3 months ago
comment:16
lancewillett
— 3 months ago
.2 prevent opening Theme Customizer.
comment:17
lancewillett
— 3 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
obenland
— 3 months ago
You're the native! Feel free to tweak it to your linking.
comment:19
lancewillett
— 3 months ago
- Owner set to lancewillett
- Resolution set to fixed
- Status changed from new to closed
In 23837:
comment:20
lancewillett
— 3 months ago
In 23838:
Related: #13780