[wp-trac] [WordPress Trac] #31723: is_page( false ) === true ?

[WordPress Trac]

#31723: is_page( false ) === true ?
------------------------------------------+----------------------
 Reporter:  Compute                       |       Owner:
     Type:  defect (bug)                  |      Status:  closed
 Priority:  normal                        |   Milestone:
Component:  Query                         |     Version:
 Severity:  normal                        |  Resolution:  wontfix
 Keywords:  needs-unit-tests 2nd-opinion  |     Focuses:
------------------------------------------+----------------------
Changes (by boonebgorges):

 * status:  new => closed
 * resolution:   => wontfix
 * milestone:  Awaiting Review =>


Comment:

 > 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 argumen 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
 * 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.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/31723#comment:12>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform