Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#20361 closed defect (bug) (fixed)

Deal with FALSE value of wp_get_theme() in ~/wp-admin/includes/dashboard.php to avoid making sad owls.

Reported by: georgestephanis's profile georgestephanis Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: Themes Keywords:
Focuses: Cc:

Description

in wp-admin/includes/dashboard.php lines 363 and 1299 wp_get_theme() is called, and its returned $theme value has a method called on it without ever being tested for whether the returned value was FALSE rather than an object.

This causes bad things. And results in sad owls. http://utterlycute.com/2012/03/sad-owl-is-so-sad/

Attachments (1)

Screen Shot 2012-04-28 at 6.51.20 PM.png (35.0 KB) - added by emiluzelac 11 years ago.

Download all attachments as: .zip

Change History (22)

#1 @nacin
11 years ago

I've been thinking about this some more. As it is going to be common to chain wp_get_theme(), I think we should consider going back to always returning an object from wp_get_theme() to avoid fatal errors. You can check wp_get_themes()->exists() or wp_get_themes()->errors() on your own.

I'll let westi weigh in on this.

#2 @nacin
11 years ago

  • Component changed from General to Themes
  • Milestone changed from Awaiting Review to 3.4

#3 @nacin
11 years ago

In [20363]:

Always return a WP_Theme object from wp_get_theme(). Check \$theme->exists() or \$theme->errors() to confirm the requested theme actually exists. see #20361.

#4 @westi
11 years ago

I like this, saves me finding more things like #20364 and makes this more backward compatible with how current_theme_info() used to behave.

#5 @nacin
11 years ago

In [20379]:

Use WP_Theme::exists(). see #20361.

#6 @nacin
11 years ago

In [20524]:

Always return a WP_Theme object from wp_get_theme(). If we can't find the theme root, assume the default theme root. (Which will probably result in a WP_Theme object that returns false for the exists() method, which is fine.) see #20361.

#7 @emiluzelac
11 years ago

  • Cc emil@… added

In case that this helps: Here is the http://screencast.com/t/oqnmmqK5B and how to recreate the error. Version now is (3.4-beta2-20521)

Update: Not sure how long will screencast.com link last, they have some monthly limits to it.

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

#8 @nacin
11 years ago

This should not be reproducible in trunk.

#9 @georgestephanis
11 years ago

Trunk is good. worksforme

#10 follow-up: @emiluzelac
11 years ago

No, not the same message, but the Theme notices such as:

  • Notice: add_custom_background is deprecated since version 3.4! Use add_theme_support( 'custom-background', $args ) instead. in /wp-includes/functions.php on line 2705
  • Notice: add_custom_image_header is deprecated since version 3.4! Use add_theme_support( 'custom-header', $args ) instead. in /wp-includes/functions.php on line 2705

will display upon Upload > Theme Activation in (3.4-beta3-20634).

Just to be clear, that's the only thing visible on the page, notices and white page nothing else.

Emil

P.S. In response to: https://twitter.com/nacin/status/196311975205150720

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

#11 in reply to: ↑ 10 ; follow-up: @jkudish
11 years ago

  • Cc joachim.kudish@… added
  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from new to closed

Replying to emiluzelac:

No, not the same message, but the Theme notices such as:

  • Notice: add_custom_background is deprecated since version 3.4! Use add_theme_support( 'custom-background', $args ) instead. in /wp-includes/functions.php on line 2705
  • Notice: add_custom_image_header is deprecated since version 3.4! Use add_theme_support( 'custom-header', $args ) instead. in /wp-includes/functions.php on line 2705

will display upon Upload > Theme Activation in (3.4-beta3-20634).

Just to be clear, that's the only thing visible on the page, notices and white page nothing else.

Emil

Neither of those 2 errors have anything to do with this ticket. Your theme is simply using two (now) deprecated functions.

#12 in reply to: ↑ 11 ; follow-up: @emiluzelac
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to jkudish:

Replying to emiluzelac:

No, not the same message, but the Theme notices such as:

  • Notice: add_custom_background is deprecated since version 3.4! Use add_theme_support( 'custom-background', $args ) instead. in /wp-includes/functions.php on line 2705
  • Notice: add_custom_image_header is deprecated since version 3.4! Use add_theme_support( 'custom-header', $args ) instead. in /wp-includes/functions.php on line 2705

will display upon Upload > Theme Activation in (3.4-beta3-20634).

Just to be clear, that's the only thing visible on the page, notices and white page nothing else.

Emil

Neither of those 2 errors have anything to do with this ticket. Your theme is simply using two (now) deprecated functions.

Not sure if I can agree with you and here's why:

Deprecated functions do not break the WordPress! I am well aware of them and they're produced by Twenty Eleven Theme and/or any other Theme with the same setup.

The "error" comes up all the time with debug true and do not break anything other than interface a little, when Theme is uploaded and activated in right away than deprecated functions will break everything.

Reopening till this is confirmed.

#13 in reply to: ↑ 12 ; follow-up: @jkudish
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to emiluzelac:

Not sure if I can agree with you and here's why:

Deprecated functions do not break the WordPress! I am well aware of them and they're produced by Twenty Eleven Theme and/or any other Theme with the same setup.

The "error" comes up all the time with debug true and do not break anything other than interface a little, when Theme is uploaded and activated in right away than deprecated functions will break everything.

Reopening till this is confirmed.

It does a white screen because of the order/timing when the errors occur. Anyhow, neither of these 2 deprecated notices have anything to do with wp_get_theme(), which is what this ticket was about in the first place.

Twenty Ten + Twenty Eleven will likely be updated prior to the 3.4 release to deal with these deprecated functions.

#14 follow-up: @emiluzelac
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#15 in reply to: ↑ 14 ; follow-up: @jkudish
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to emiluzelac:

See: https://twitter.com/nacin/status/196311975205150720

I'm not arguing with you that you are getting errors. Just clarifying that the errors you are getting have nothing to do with the initial ticket, and simply side-effects of Twenty Ten/Eleven not being updated for 3.4 yet.

#16 in reply to: ↑ 15 ; follow-up: @emiluzelac
11 years ago

Replying to jkudish:

Replying to emiluzelac:

See: https://twitter.com/nacin/status/196311975205150720

I'm not arguing with you that you are getting errors. Just clarifying that the errors you are getting have nothing to do with the initial ticket, and simply side-effects of Twenty Ten/Eleven not being updated for 3.4 yet.

I understand deprecated notices and of course they will be changed in 3.4 no doubt about that, but that wasn't my point at all. Anyone with the basic knowledge of WordPress will tell you that deprecated notices will not produce "white screen" upon Theme upload/activation. Notice: Undefined index: can but not deprecated functions.

Since 12/10 I reviewed over 830 Themes (WPTRT - not bragging just giving you more details) and not a single Theme ever "broke" the WordPress nor created a 'white page" just because they were using deprecated functions, regardless how the Theme was activated.

I am not going to reopen the ticket because I am not sure if this is indeed ticket related or not, will wait for Andy and you would close it anyways without real explanation :)

Emil

#17 @jkudish
11 years ago

I agree with you, themes shouldn't break the theme activator. I also agree with you that the errors should be fixed (and I'm sure they will). However, the errors are breaking the activator because of when in the code load order they occur + it has nothing to do with this ticket, so if you really want to report it, it belongs in a new ticket, IMO. Hope that clarifies my point of view.

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

#18 in reply to: ↑ 13 @SergeyBiryukov
11 years ago

Replying to jkudish:

Twenty Ten + Twenty Eleven will likely be updated prior to the 3.4 release to deal with these deprecated functions.

#20265

#19 in reply to: ↑ 16 @nacin
11 years ago

Replying to emiluzelac:

I understand deprecated notices and of course they will be changed in 3.4 no doubt about that, but that wasn't my point at all. Anyone with the basic knowledge of WordPress will tell you that deprecated notices will not produce "white screen" upon Theme upload/activation. Notice: Undefined index: can but not deprecated functions.

The point that jkudish was trying to make is that they were unrelated, because they were.

The deprecated notices did not cause a whitescreen. It was apparent that a fatal error (undefined method exists()) caused the white-screen. There also just happened to be unrelated notices on the page. They did not prevent the activation.

You were testing on [20521]. This was fixed in [20524]. I wanted confirmation that it was fixed for you. The expectation was that it was and you would not be able to break it again. :-)

Thanks for your help, all of you. Leaving as fixed.

#20 follow-up: @nacin
11 years ago

will display upon Upload > Theme Activation in (3.4-beta3-20634).
Just to be clear, that's the only thing visible on the page, notices and white page nothing else.

This I could not reproduce. Emil, if you can, please open a new ticket.

#21 in reply to: ↑ 20 @emiluzelac
11 years ago

Replying to nacin:

will display upon Upload > Theme Activation in (3.4-beta3-20634).
Just to be clear, that's the only thing visible on the page, notices and white page nothing else.

This I could not reproduce. Emil, if you can, please open a new ticket.

First thanks for the previous explanations and yes I can reproduce and will open new ticket too.

Emil

Note: See TracTickets for help on using tickets.