Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#23837 closed defect (bug) (fixed)

Twenty Ten: Fatal error in loop.php

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

Description

A fatal error is being fired on twentyten's loop.php file, gallery portion:

wp-content/themes/pub/twentyten/loop.php:96 - Object of class WP_Error could not be converted to string

On line 96 of loop.php we should be able to output the category link. But if the term we're looking up doesn't exist, get_term_link returns a WP_Error, and echo'ing that get_term_link throws the fatal error quoted above.

I've attached a patch that checks if it's "get_term_link" is returning a WP_Error.

This has been reported previously: http://wordpress.org/support/topic/get_term_link-throwing-error

Attachments (4)

twentyten-gettermlink-fix.diff (1.3 KB) - added by keoshi 11 years ago.
23837.diff (1.9 KB) - added by lancewillett 11 years ago.
23837.2.diff (1.1 KB) - added by SergeyBiryukov 11 years ago.
23837.3.diff (1.5 KB) - added by SergeyBiryukov 11 years ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
11 years ago

  • Component changed from Themes to Bundled Theme

#2 @SergeyBiryukov
11 years ago

  • Summary changed from Fatal error on TwentyTen's loop.php to Twenty Ten: Fatal error in loop.php

#3 @lancewillett
11 years ago

  • Milestone changed from Awaiting Review to 3.6

Shouldn't the fix be in the core get_term_link() function instead? Seems weird to check for an error object in a theme, to me.

#4 @lancewillett
11 years ago

Possibly related to r17438 and #16521 ?

#5 follow-up: @lancewillett
11 years ago

Oh, wait -- "Restore WP_Error return to get_term_link()" in r17437. :|

#6 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
11 years ago

Replying to lancewillett:

Oh, wait -- "Restore WP_Error return to get_term_link()" in r17437. :|

Yes, it's documented that get_term_link() returns a WP_Error object if the term doesn't exist:
http://codex.wordpress.org/Function_Reference/get_term_link#Return_Values

Seems like Twenty Ten should either use get_category_link(), which still returns an empty string on error (but only accepts category ID or object), or add is_wp_error() check.

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

#7 in reply to: ↑ 6 @lancewillett
11 years ago

Replying to SergeyBiryukov:

Replying to lancewillett:

Oh, wait -- "Restore WP_Error return to get_term_link()" in r17437. :|

Seems like Twenty Ten should either use get_category_link(), which still returns an empty string on error (but only accepts category ID or object), or add is_wp_error() check.

Template tags should never return WP_Error - I think that should be a rule
That seems prudent. WordPress themes are popular partly because of simplicity. Handling error objects isn't simple.

Via https://core.trac.wordpress.org/ticket/16521#comment:4

I tend to agree with that. So maybe switching to get_category_link is the way to go.

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

#8 @lancewillett
11 years ago

Hmm ... we need the category ID, that's why we weren't using it before. get_category_link() doesn't allow passing a string "slug" to find the category URL.

@lancewillett
11 years ago

#9 follow-up: @lancewillett
11 years ago

  • Keywords has-patch added

Patch 23837.diff allows passing a string to get_category_link() -- we don't need to check type here because get_term_link() already does that: it accepts object, int, or string.

#10 in reply to: ↑ 9 @SergeyBiryukov
11 years ago

Replying to lancewillett:

Patch 23837.diff allows passing a string to get_category_link() -- we don't need to check type here because get_term_link() already does that: it accepts object, int, or string.

See ticket:16521:10 for the reason behind casting to int. Removing that would cause back compat issues similar to the ones in #22324.

#11 @lancewillett
11 years ago

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

In 23793:

Twenty Ten: change gallery category post meta output to check for a "gallery" category using get_term_by() to avoid a possible WP_Error for an empty result by getting a false (boolean) return value instead.

And, change link output to use get_category_link() instead of get_term_link().

Props SergeyBiryukov and keoshi, fixes #23837.

Note: See TracTickets for help on using tickets.