is_page( false ) === true ?

[Compute]

​https://developer.wordpress.org/reference/classes/wp_query/is_page/

Is the query for an existing single page?

But this really does not make sense to me:

if ( empty( $page ) )
    return true;

Is there a reason that is_page() will return true on empty, false, 0 values?

[SergeyBiryukov]

Introduced in [1728].

[valendesigns]

Unfortunately, if we were to make that simple change and return false then 36 tests fail. I do agree that it seems wrong that it would return true in these situations, but doing anything about it would require a lot of refactoring of tests and could have wide sweeping backwards compatibility implications. I'm inclined to close as wontfix because of it.

[jipmoors]

Replying to valendesigns:

Unfortunately, if we were to make that simple change and return false then 36 tests fail. I do agree that it seems wrong that it would return true in these situations, but doing anything about it would require a lot of refactoring of tests and could have wide sweeping backwards compatibility implications. I'm inclined to close as wontfix because of it.

I disagree, a simple modification on the default value would solve this problem.
The way it is set up now is pretty ugly and can simply be improved upon by setting the default to null. This will keep all the tests passing and prevents bad data from being entered.

[jipmoors]

to null change on default value

[valendesigns]

That's an interesting approach. I didn't think about setting it to null. Though it still seems odd that a null value would return true.

[jipmoors]

No it's quite logical; null means that it will respect the $this->is_page value.
The is_null check will only be ran when $this->is_page is not false.

It would be more readable if it would say return (bool) $this->is_page but that is basically what it does without the use of the variable.

If we don't have some custom object, list or ID to check against, the global query is used which already determined if it is a page.

[jipmoors]

Actually. Looking through the file, some other functions could use this treatment aswel. Might be good to check if there was a decision not to use this approach or some ticket about this is still opened.

In each case the patch is incomplete.

[boonebgorges]

What exactly is the bug here, aside from the fact that it seems weird? Did an actual situation come up where you were passing false or 0 to is_page()?

See #24674 [31458] for a related issue.

[Compute]

I have an option that filters the query:

if ( is_page( get_option( 'special_filter' ) ) {
    $query->set( 'special_filter', 1 );
}

Above works if the option has been set (as the ID might be 54).

But if the option has not been set it returns true on all pages. The fix is really to check for the option before passing it into the is_page() check:

$option = get_option( 'special_filter' );

if ( ! $option ) {
    return $query;
}

if ( is_page( $option ) {
    $query->set( 'special_filter', 1 );
}

To me it just seems wierd that is_page( false ) is true. If this is really working as intended then the description should be updated.

I truely understand if it's a wontfix, I was just wondering WHY this makes any sense.

[jipmoors]

While testing code: returning true or false on empty(ish) input mostly doesn't matter; though one of the two should be an unwanted return value.

The question is: do we want to invalidate unusable values or assume 'current' item variables apply

Logic dictates that we invalidate anything other than an empty call, because the default has been overridden.
If this is applied to all is_* functions in query.php only one test breaks: entering an empty array for is_tag is_tag(array()) which expects to be true.

I can understand why that test has been applied; and it should be removed or applied to all other functions aswel.
There are no comments on the test and situations this is created for.

Feedback requested on validity of this test

Writing the needed tests will be possible when the goal/desire/expectations are clearer.

Suggested 'template' for functions with one parameter:

function is_param($param = null) {
  if ( is_null( $param ) || ! $this->is_param )
	return (bool) $this->is_param;

  $param = (array) $param;

  if ( empty( $param ) )
	return true;

  ... code ...

  return false
}

Changes:

• Changed default value of parameter from '' to null.
• Moved the casting to array up in the function.
• Returning true on empty (array).

This way an empty array can be entered for default behaviour, anything else will be made into an array value so page titles can have '0' and be found.

All checks inside the function return 'true' except the first and last ones. Which provides much more readable code and debugging.

If we want to invalidate the empty array the only thing to do is swap the array casting and empty check.

I have a patch with this applied to all functions and they pass all current tests.

[jipmoors]

applied to all functions + added unit tests for invalid data

[boonebgorges]

I truely understand if it's a wontfix, I was just wondering WHY this makes any sense.

In a nutshell: is_page() (and is_author(), etc) is intended to be used in two different ways. One is to pass a specific ID or slug: is_page( 'foo' ), is_page( 123 ). The other is the more general check "is this a page at all?", where no param is passed: is_page(). So the logic of these functions goes something like this:

• If parse_query() has already determined that this is not a page query at all (https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/query.php?marks=1598,1599#L1587), return false early https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/query.php?marks=4339,4340#L4324.
• If empty( $page ), we assume that no argument has been passed to is_page(), and we're simply checking to see whether we're on any sort of page at all. We already know that we are, after the previous step. So we return true. https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/query.php?marks=4342,4343#L4324 This assumption is the source of the bug - empty( $page ) is too broad a check.
• From this point on, we compare the passed value of $page against the current page.

If we were designing the system from scratch, something like jipmoors's suggestion would be good. Or even something more explicit, using argument overloading:

public function is_page() {
    if ( ! func_num_args() ) {
        return $this->is_page;
    }

    $page = func_get_arg( 0 );
    // rest of the function
}

31723.1.diff​ fixes the behavior for is_page( 0 ) and is_page( false ) and is_page( '' ), but not for is_page( null ). I suppose there's an argument for privileging '' and false: get_metadata() will generally return an empty string when the requested value is not found, and get_option() generally returns false, which means that the bug'll be fixed for cases where a falsey value is stored in one of these two places. My func_num_args() trick is a broader fix, though I know that in the past, function argument overloading has been frowned upon (harder to read and document).

In any of these cases, backward compatibility is a concern. It's very likely that plugins are counting on is_page( 0 ) to return the same as is_page(). It's a hard thing to verify, though - I'm not sure how to search for it.

I'm leaning toward wontfix. As Compute notes, there is an easy workaround, and you might argue that plugin authors ought to be doing this kind of validation anyway. If anyone has a powerful practical argument that wontfix is the wrong way forward here, please feel free to reopen with details.

[SergeyBiryukov]

#32267 was marked as a duplicate.