Opened 8 years ago
Last modified 10 months ago
#24142 reviewing defect (bug)
Zero value for posts_per_page value in wp_query custom instance and for 'Blog pages show at most' option
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Query | Keywords: | needs-unit-tests needs-testing |
Focuses: | Cc: |
Description
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)
Change History (14)
#2
in reply to:
↑ 1
@
8 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...
#4
@
8 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
@
8 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
@
6 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
@
6 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
@
4 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
@
3 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,
https://core.trac.wordpress.org/browser/tags/4.8.2/src/wp-includes/class-wp-query.php#L2781
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:
https://core.trac.wordpress.org/browser/tags/4.8.2/src/wp-includes/class-wp-query.php#L1785-L1798
i.e.
$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.
#10
@
12 months 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.
What's the use case of having a query which should return 0 results?