WordPress.org

Make WordPress Core

#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 13 months ago.
23837.diff (1.9 KB) - added by lancewillett 13 months ago.
23837.2.diff (1.1 KB) - added by SergeyBiryukov 13 months ago.
23837.3.diff (1.5 KB) - added by SergeyBiryukov 13 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 SergeyBiryukov13 months ago

  • Component changed from Themes to Bundled Theme

comment:2 SergeyBiryukov13 months ago

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

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

Possibly related to r17438 and #16521 ?

comment:5 follow-up: lancewillett13 months ago

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

comment:6 in reply to: ↑ 5 ; follow-up: SergeyBiryukov13 months 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 13 months ago by SergeyBiryukov (previous) (diff)

comment:7 in reply to: ↑ 6 lancewillett13 months 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 13 months ago by lancewillett (previous) (diff)

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

lancewillett13 months ago

comment:9 follow-up: lancewillett13 months 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.

SergeyBiryukov13 months ago

SergeyBiryukov13 months ago

comment:10 in reply to: ↑ 9 SergeyBiryukov13 months 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 lancewillett13 months 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.