WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#23837 closed defect (bug) (fixed)

Twenty Ten: Fatal error in loop.php

Reported by: keoshi Owned by: 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 2 years ago.
23837.diff (1.9 KB) - added by lancewillett 2 years ago.
23837.2.diff (1.1 KB) - added by SergeyBiryukov 2 years ago.
23837.3.diff (1.5 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 @SergeyBiryukov2 years ago

  • Component changed from Themes to Bundled Theme

comment:2 @SergeyBiryukov2 years ago

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

comment:3 @lancewillett2 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.

comment:4 @lancewillett2 years ago

Possibly related to r17438 and #16521 ?

comment:5 follow-up: @lancewillett2 years ago

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

comment:6 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov2 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() can return a WP_Error object if the term doesn't exist:
http://codex.wordpress.org/Function_Reference/get_term_link#Return_Values

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

Version 0, edited 2 years ago by SergeyBiryukov (next)

comment:7 in reply to: ↑ 6 @lancewillett2 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 2 years ago by lancewillett (previous) (diff)

comment:8 @lancewillett2 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.

@lancewillett2 years ago

comment:9 follow-up: @lancewillett2 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.

@SergeyBiryukov2 years ago

@SergeyBiryukov2 years ago

comment:10 in reply to: ↑ 9 @SergeyBiryukov2 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.

comment:11 @lancewillett2 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.