Make WordPress Core

Opened 12 years ago

Last modified 7 years ago

#21790 assigned defect (bug)

When set a static front page WP main query isn't set correctly

Reported by: markoheijnen's profile markoheijnen Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4.1
Component: Query Keywords: needs-patch
Focuses: Cc:

Description

In my project I use on several places pre_get_posts filter. When setting a static frontpage and blog page I get several notices on my screen. When I var_dump the main query the only value that is set it the page_id.
Even the post_type isn't set.

Attachments (1)

21790.diff (1.7 KB) - added by collinsinternet 9 years ago.
Unit test that catches PHP notice on pre_get_post when static front page is set.

Download all attachments as: .zip

Change History (31)

#1 follow-up: @nacin
12 years ago

The notices are?

#2 in reply to: ↑ 1 @markoheijnen
12 years ago

Replying to nacin:

The notices are?

Notice: Undefined property: WP_Query::$post in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/query.php on line 2986

Notice: Trying to get property of non-object in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/post-template.php on line 1270

Notice: Undefined property: WP_Query::$post in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/query.php on line 2986

Notice: Trying to get property of non-object in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/query.php on line 3349

Notice: Trying to get property of non-object in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/query.php on line 3351

Notice: Trying to get property of non-object in /var/www/vhosts/*/subdomains/*/httpdocs/wp-includes/query.php on line 3353

#3 follow-up: @markoheijnen
12 years ago

In my case I'm using is_page_template() and is_front_page() on the filter. How I look in the code this will only work inside the loop but I can be wrong. However I somehow do expect it to work without the need of a loop.

#4 @nacin
12 years ago

Could you include a simple test case that can reproduce this?

#5 @markoheijnen
12 years ago

For me to see the notices is to set a static blog and front page. and use a function like is_front_page. I'm going to try if I can write an unit-test to make it visible. Not sure yet how to create this situation and where to check on.

function some_random_modification( $query ) {
		if( $query->is_main_query() && is_front_page() ) {
		}

		return $query;
}
add_filter( 'pre_get_posts', 'some_random_modification' );

#6 in reply to: ↑ 3 @SergeyBiryukov
12 years ago

is_front_page() calls is_page(), which only works on 'wp' action or later.

'pre_get_posts' is too early to use conditional tags (this is also mentioned on the Codex page).

#7 @markoheijnen
12 years ago

Uhm okay, i do think it is wrong tho. 'pre_get_posts' is a place you want to use it since you want to control the main query.

How I can see it now there is no way to do it then. Having said that it does work. I only get some notices.

#8 @markoheijnen
12 years ago

Just a little note what I already said: it only shows the notices when static homepage is set. On all other pages it is fine without any notice.

#9 @SergeyBiryukov
12 years ago

Well, is_front_page() only calls is_page() when a static home page is set:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/query.php#L3287

The notices are coming from get_queried_object(), where $this->post is not filled yet:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/query.php#L2986

So $page_obj in is_page() ends up being null:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/query.php#L3345

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

#10 @markoheijnen
12 years ago

yes, I know that. I looked through the code to see if I could fix it.

I'm now just curious if it is fixable or how you otherwise modify the query on the front page with the use of WordPress code.

#11 @markoheijnen
12 years ago

I now solved the issue by using this:

( $query->get('page_id') == get_option('page_on_front') || is_front_page() )

Reason was that is_front_page() doesn't work then in pre_get_posts(). It does work but I'm not a fan of using this.

#12 @wonderboymusic
11 years ago

  • Keywords dev-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This ticket appears to be dead - reopen if you think this needs a fix, Marko

#13 @markoheijnen
11 years ago

Ticket is dead since I have no clue how to fix it. The problem is that even core developers say you can use is_front_page() or is_home on the hook pre_get_posts but you can't since it needs the page obj if a static page has been set. It also will fail then.

So no clue if the code should be fixed or the story we tell to developers.

#14 @markoheijnen
11 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Reopening the ticket. This needs a code fix or documentation fix.

#15 @SergeyBiryukov
11 years ago

  • Milestone set to Awaiting Review

#16 @wonderboymusic
10 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.0

This seems tangentially related to #27015 - will review both

#17 @helen
10 years ago

  • Milestone changed from 4.0 to Awaiting Review

#27015 isn't happening in 4.0.

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


10 years ago

#19 @SergeyBiryukov
10 years ago

#28728 was marked as a duplicate.

#20 @SergeyBiryukov
9 years ago

  • Keywords 4.2-early added
  • Milestone changed from Awaiting Review to Future Release

Just got bitten by this, trying to use is_front_page() in pre_get_posts.

Looks like the patch on #27015 should resolve this.

#21 @obenland
9 years ago

  • Owner set to SergeyBiryukov
  • Status changed from reopened to assigned

#22 @obenland
9 years ago

  • Keywords 4.2-early removed
  • Milestone changed from Future Release to 4.3

@collinsinternet
9 years ago

Unit test that catches PHP notice on pre_get_post when static front page is set.

#23 @collinsinternet
9 years ago

  • Keywords needs-unit-tests removed

This ticket was mentioned in Slack in #core by jorbin. View the logs.


9 years ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


9 years ago

#26 @jorbin
9 years ago

  • Milestone changed from 4.3 to Future Release

No traction in 4.3, punting.

#27 @lkraav
8 years ago

Anybody able to pick this up for 4.8 or so?

#28 @justnorris
8 years ago

This has been around for a while now. It would be really great to se some movement for 4.8.

This ticket was mentioned in Slack in #core by mattkersley. View the logs.


7 years ago

#30 @Timbre Design
7 years ago

Agreed @justnorris I've noticed quite a few folks having issue with this.

Per @jkohlbach it looks like he's found a workaround in the meantime (this is a WooCommerce specific example but will work elsewhere):

<?php
add_action( 'pre_get_posts', 'is_shop_workaround_demo', 1 );

function is_shop_workaround_demo( $query ) {
    $front_page_id        = get_option( 'page_on_front' );
    $current_page_id      = $query->get( 'page_id' );
    $shop_page_id         = apply_filters( 'woocommerce_get_shop_page_id' , get_option( 'woocommerce_shop_page_id' ) );
    $is_static_front_page = 'page' == get_option( 'show_on_front' );

    // Detect if it's a static front page and the current page is the front page, then use our work around
    // Otherwise, just use is_shop since it works fine on other pages
    if ( $is_static_front_page && $front_page_id == $current_page_id  ) {
        error_log( 'is static front page and current page is front page' );
        $is_shop_page = ( $current_page_id == $shop_page_id ) ? true : false;
    } else {
        error_log( 'is not static front page, can use is_shop instead' );
        $is_shop_page = is_shop();
    }

    // Now we can use it in a conditional like so:
    if ($is_shop_page) {
        error_log( 'this is the shop page' );
    }
}

https://github.com/woocommerce/woocommerce/issues/10625

Last edited 7 years ago by Timbre Design (previous) (diff)
Note: See TracTickets for help on using tickets.