Previous fix for get_queried_object, gives notices for tax queries

[layotte]

Seeing this notice when browsing to a taxonomy archive page (several times):
'Notice: Undefined variable: query in /home/lew/public_html/wp-includes/query.php on line 3274'

Changed in my patch to $this->is_tax, since $query isn't defined anywhere beforehand.

related [26864], #26634, #26627.

[Otto42]

Makes sense. $query is the wrong thing to use there.

[Chouby]

I believe that at this point $this->is_tax is always true. So the proposed patch should reintroduce the bug corrected in [26864]. I thus propose to test for:

empty($query['terms'])

instead of only

$query['terms']

That would be strange enough if this notice was not present in WP < 3.8 since [26864] reintroduced the test exactly as it was before [26007].

[layotte]

If $this->is_tax is always true, than the line 3265 above will always be true:

 if ( $this->is_category || $this->is_tag || $this->is_tax )

And $query is never defined in this function, so it'll always be empty.

[Otto42]

Seems like this code had a bug to begin with then, as $query is undefined in this function, and $query[ 'terms' ] will never have any content.

Rather than reintroducing a bug, let's determine what the code should be doing here. I cannot see why is_tax would always be true here, except insofar as the check before it was only checking for those cases.

[Otto42]

26728.diff looks good, but I don't think the check for if ( $query[ 'terms' ] ) is necessary. The field, terms, and taxonomy should all be set at that point from the $tax_query_in_and.

[layotte]

That's almost what was in [26007]... then in [26864] the elseif ( $query['terms'] ) was added. Perhaps wonderboymusic can shed some light (I don't know if it's possible to tag someone in a trac post).

[layotte]

For what it's worth, $this->is_tax is not always true at this point (from my quick testing).

[SergeyBiryukov]

Seems like 26728.diff​ would be the correct fix there.

Replying to Chouby:

That would be strange enough if this notice was not present in WP < 3.8 since [26864] reintroduced the test exactly as it was before [26007].

Not exactly, $query was defined at that point before [26007] (see line 3253 on the left).

Replying to layotte:

If $this->is_tax is always true, than the line 3265 above will always be true:

No, it's vice versa. If we're inside that condition, then either $this->is_category, $this->is_tag, or $this->is_tax is true. We check for $this->is_category and $this->is_tag above, so if they are both false, that means $this->is_tax is true.

Replying to Otto42:

26728.diff looks good, but I don't think the check for if ( $query[ 'terms' ] ) is necessary.

Related: [22450], #26634.

[layotte]

Replying to SergeyBiryukov:

Replying to layotte:

If $this->is_tax is always true, than the line 3265 above will always be true:

No, it's vice versa. If we're inside that condition, then either $this->is_category, $this->is_tag, or $this->is_tax is true. We check for $this->is_category and $this->is_tag above, so if they are both false, that means $this->is_tax is true.

Ah, yes, misunderstood what he meant.

+1 to 26728.diff

[wonderboymusic]

In 26874:

In WP_Query::get_queried_object(), move the check for $query['terms'] to a place where $query is actually set. This should be included if [26864] makes it into 3.8.1.

Props SergeyBiryukov.
See #26728, [26864], #26634, #26627.

[SergeyBiryukov]

26728.test.diff​ fails before [26874] and passes after it.

[SergeyBiryukov]

In 26875:

Add unit test for [26874]. see #26728.

[Chouby]

Replying to SergeyBiryukov:

Not exactly, $query was defined at that point before [26007] (see line 3253 on the left).

This seems to be what we all missed in previous attempts to solve #26634

Should we close #26634 and mark it as duplicate in favor of this ticket as now the right fix is dealt here?

[SergeyBiryukov]

Replying to Chouby:

Should we close #26634 and mark it as duplicate in favor of this ticket as now the right fix is dealt here?

I think we can close this one as fixed in 3.9, and keep both #26627 and #26634 open (as they are focused on different issues) for merging the changes to 3.8.1.