Make WordPress Core

Opened 7 years ago

Last modified 4 weeks ago

#24142 reopened defect (bug)

Zero value for posts_per_page value in wp_query custom instance and for 'Blog pages show at most' option

Reported by: alexvorn2 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Query Keywords: needs-unit-tests needs-testing
Focuses: Cc:


To show no posts if the posts_per_page value is 0.

Currently for custom instances of wp_query, if the value is 0 then this is changed with the value from posts_per_page option from the database. "get_options( 'posts_per_page' )"

For home page if we set value 0 on the settings page, in wp-admin/options-reading.php, after the saves are changed, this value is changed to 1.

I think for both cases if the posts per page value is 0 then no posts should not display.

Attachments (1)

24142.diff (3.7 KB) - added by Howdy_McGee 4 weeks ago.
Removal of empty() in favor of isset() is_numeric() and is_bool()

Download all attachments as: .zip

Change History (11)

#1 follow-up: @ocean90
7 years ago

  • Keywords reporter-feedback close added

What's the use case of having a query which should return 0 results?

#2 in reply to: ↑ 1 @alexvorn2
7 years ago

Replying to ocean90:

What's the use case of having a query which should return 0 results?

To hide all posts, for example for a short period of time if the site is under construction...

#3 @alexvorn2
7 years ago

  • Keywords needs-docs added; reporter-feedback removed

#4 @wonderboymusic
7 years ago

  • Keywords close needs-docs removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Why would you write a WP_Query by hand that wants zero posts? Why not just comment out the code that displays posts or something

#5 @DrewAPicture
7 years ago

A more valid use case for this might be if you're passing the post_per_page value through absint() or similar and zero gets returned.

I can follow the logic of -1 = unlimited, or 1 = 1, 2 = 2 and so on. So it's odd that we say, "Oh, by the way, 0 doesn't equal 0 here, it actually equals this other thing over there."

It kind of stands to reason that if I pass 0 I'd expect to get 0 results, intentional or not. Just sayin'.

#6 @randell
5 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I agree with @DrewAPicture's logic on how zero is handled relative to how -1, 1, 2, 3, etc. is handled. Wasted so much time trying to figure out what's wrong with my code because of this unintuitive behavior.

#7 @boonebgorges
5 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone set to Future Release

I agree that it's a bit odd to create a WP_Query with posts_per_page=0, but I also think there's no reason we shouldn't support it. The culprit appears to be https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-includes/query.php#L2460. Something like 0 === $q['posts_per_page'] || '0' === $q['posts_per_page'] is probably going to work, but this needs unit tests.

#8 @teraviva
3 years ago

I want to give you an other case. For example I have CPT Destination with Tours and Posts attached to it. And I have no need to show posts at Destination page, because I provide further links to inner categories for Tours and Info posts at this page.

#9 @birgire
2 years ago

I agree that this is a strange behavior.

To better understand it, we can map it out for version 4.8.2:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
null	|	default		default		default			default
''	|	default		default		default			default
0	|	default		default		default			default
'0'	|	default		default		default			default
true	|	none		1		none			1
false	|	default		default		default			default
10	|	10		10		10			10
'10'	|	10		10		10			10
'string'|	1		1		default			1
array() |	default		default		1			default
-1	|	none		none		none			none
-2 	|	2		2		2			2

where default means the stored option's value. We could e.g. use this table when writing tests and data providers.

So there are some strange inconsistencies, e.g. the true row ;-)

The scope? of the ticket seems to be adjust the the 0 and '0' rows to:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
0	|	0		0		0			0
'0'	|	0		0		0			0

I think these two row cases, should be the same as returning an empty array from
the posts_pre_query filter,


so we don't have to run those db queries. Like using $this->posts = array().

We would need to check for e.g.

if( isset( $q['posts_per_page'] ) && ( 0 === $q['posts_per_page'] || '0' === $q['posts_per_page'] ) ) {

as mentioned by @boonebgorges and similar for the other query variables.

We would also have to adjust this part accordingly:



$q['posts_per_page'] = (int) $q['posts_per_page'];

if ( $q['posts_per_page'] < -1 )
	$q['posts_per_page'] = abs($q['posts_per_page']);
elseif ( $q['posts_per_page'] == 0 )
	$q['posts_per_page'] = 1;

The case of e.g. 'showposts' => 'string' is a little bit tricky here to detect, because of the previous $q['showposts'] = (int) $q['showposts'];

Before writing any patches I think we should consider what the scope of this ticket should be,
i.e. should it be to only adjust the 0 and '0' rows or if it should also touch the true, false, 'string', array(), ... rows?

Hope this helps anyone that's going to tackle this problem.

Update: Then there's ticket #39945 for yet another edge case input value.

Last edited 2 years ago by birgire (previous) (diff)

#10 @Howdy_McGee
4 weeks ago

  • Keywords needs-testing added; needs-patch removed

I think we can get away with == if we convert the given value to integer after checking if we need to provide a default. Similar to the showposts conditional directly below pre_get_posts.

Giving posts_per_page a TRUE boolean already translates to 1 (post per page) so it would make sense for FALSE to translate to 0 and return 0 posts.

This certainly needs tested but from what I've run it works as expected. To compare to the previously listed chart:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
null	|	default		default		default			default
''	|	default		default		default			default
0	|	0		0		0			0
'0'	|	0		0		0			0
true	|	1		1		1			1
false	|	0		0		0			0
10	|	10		10		10			10
'10'	|	10		10		10			10
'string'|	default		default		default			default
array() |	default		default		default			default
-1	|	all		all		all			all
-2 	|	2		2		2			2

I think in the end it makes posts_per_page a bit more predictable for edges cases such as this. The biggest change users should be aware of is how booleans passed to posts_per_page works.

4 weeks ago

Removal of empty() in favor of isset() is_numeric() and is_bool()

Note: See TracTickets for help on using tickets.