Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#26627 closed defect (bug) (fixed)

Regression: get_queried_object no longer working with pre_get_posts and pretty permalinks

Reported by: otto42's profile Otto42 Owned by: nacin's profile nacin
Milestone: 3.8.1 Priority: normal
Severity: normal Version: 3.8
Component: Query Keywords: has-patch needs-testing fixed-major
Focuses: Cc:

Description

Plugin code to demonstrate the issue:

function test( $query ) {
    if ( ! $query->is_main_query() )
    return;

    if ( $query->is_category('uncategorized') ) {
        var_dump($query->get_queried_object());
        die;
    }
}
add_action( 'pre_get_posts', 'test' );

Now visit /category/uncategorized on a default install with any form of non-default (pretty) permalinks configured.

Result in 3.7, it dumps this object and dies:

object(stdClass)#95 (15) { ["term_id"]=> &string(1) "1" ["name"]=> &string(13) "Uncategorized" ["slug"]=> &string(13) "uncategorized" ["term_group"]=> string(1) "0" ["term_taxonomy_id"]=> string(1) "1" ["taxonomy"]=> string(8) "category" ["description"]=> &string(0) "" ["parent"]=> &string(1) "0" ["count"]=> &string(1) "1" ["cat_ID"]=> &string(1) "1" ["category_count"]=> &string(1) "1" ["category_description"]=> &string(0) "" ["cat_name"]=> &string(13) "Uncategorized" ["category_nicename"]=> &string(13) "uncategorized" ["category_parent"]=> &string(1) "0" }

Result in 3.8: The is_category check returns false and no object is dumped.

Removing the is_category check reveals that the get_queried_object returns NULL.

Note that switching to default (ugly) permalinks restores the expected behavior. Also note that using the ?cat=1 style URL gives the expected behavior even with pretty permalinks enabled.

Attachments (3)

26627.diff (616 bytes) - added by jeremyfelt 10 years ago.
26627.2.diff (2.5 KB) - added by wonderboymusic 10 years ago.
26627.3.diff (2.2 KB) - added by wonderboymusic 10 years ago.

Download all attachments as: .zip

Change History (25)

#1 @Otto42
10 years ago

Relevant support thread where people had problems because of this:
http://wordpress.org/support/topic/pre_get_posts-doenst-work-with-pretty-permalinks-anymore

#2 @Otto42
10 years ago

Issue appears to have been introduced in [26007] for #20767.

#3 @jeremyfelt
10 years ago

At first glance, tags appear to be safe. Both tag_id and tag are available as query_vars. Confirmed on categories.

@jeremyfelt
10 years ago

#4 @jeremyfelt
10 years ago

  • Keywords has-patch needs-testing added

26627.diff checks for a valid term after using cat and then makes an attempt with get_term_by() using category_name if it is not found.

#5 @wonderboymusic
10 years ago

  • Milestone changed from Awaiting Review to 3.9

26627.2.diff is a different take and expands on my original unit tests - in most cases involving tax_query, the state of query vars is different during pre_get_posts then it is after query has run.

For instance, get_query_var( 'taxonomy' ) and get_query_var( 'term_id' ) contain values, but $query->get( 'taxonomy' ) and $query->get( 'term_id' ) in pre_get_posts do not

#6 @jeremyfelt
10 years ago

  • Milestone changed from 3.9 to 3.8.1

26627.2.diff works for me. Tests fail before, pass afterward. Reported issue is clear.

Moving to 3.8.1 for consideration.

#7 @lkraav
10 years ago

  • Cc leho@… added

#8 @wonderboymusic
10 years ago

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

Fixed in [26864].

#9 @nacin
10 years ago

  • Keywords fixed-major added

Re-opening to be code-reviewed and considered for inclusion in 3.8.1.

#10 @nacin
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#11 @layotte
10 years ago

Submitted patch here #26728 which fixes bug in [26864]

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

#12 @Otto42
10 years ago

Related: #26728

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


10 years ago

#14 @nacin
10 years ago

[26874] fixes the bug in [26864].

We usually default to reverting changes that result in regressed behavior in a minor release, opting to look forward to fixing the issue in the next major release. This prevents us from digging ourselves into a bigger hole with an additional regression (not unlike like the bug caused by [26864]).

wonderboymusic and SergeyBiryukov, to be clear: For 3.8.1, the recommendation is to merge [26864] and [26874], in lieu of reverting [26007]?

#15 @wonderboymusic
10 years ago

yes, please

#16 @SergeyBiryukov
10 years ago

Yes. Working on a unit test for [26874].

#17 @SergeyBiryukov
10 years ago

Unit test committed in [26875].

#18 @jeremyclarke
10 years ago

  • Cc jer@… added

Hit this problem really hard and [26864] solved it for me. Definitely should be in 3.8.1 IMHO.

#19 @nacin
10 years ago

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

In 26946:

Query: Fix category handling in pre_get_posts when pretty permalinks are in use (category_name query variable).

Merges [26864] [26874] [26875] to the 3.8 branch.

props wonderboymusic, SergeyBiryukov.
fixes #26627.

#20 @wonderboymusic
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

26627.3.diff makes get_queried_object() work for tags in pre_get_posts - note, tags do not get rolled up into tax_query, categories do.

#21 @nacin
10 years ago

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

This ticket was closed on a completed milestone. I'm re-closing.

Note: See TracTickets for help on using tickets.