WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 12 hours ago

#16802 new enhancement

is_page() doesn't accept a full path

Reported by: johnbillion Owned by:
Milestone: 4.0 Priority: normal
Severity: minor Version: 3.1
Component: Query Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

If you have a page at example.com/foo/bar, then is_page('foo/bar') will return false on that page.

The is_page() function only checks the ID, title and post name, not the full path.

If the $page parameter passed to is_page() contains a slash then we should check the full path. If I get a chance I'll see if I can whip up a patch in the next couple of days.

Attachments (2)

16802.patch (641 bytes) - added by johnbillion 3 years ago.
16802.2.patch (789 bytes) - added by Jesper800 7 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 johnbillion3 years ago

  • Cc johnbillion@… added

comment:2 follow-up: hakre3 years ago

I suggest to name such a function is_page_path('foo/bar') for a starter to prevent further parameter overloading of is_page().

comment:3 in reply to: ↑ 2 johnbillion3 years ago

Replying to hakre:

I suggest to name such a function is_page_path('foo/bar') for a starter to prevent further parameter overloading of is_page().

What's "parameter overloading"? Why do we need another function?

comment:4 greuben3 years ago

  • Keywords 2nd-opinion added

is_page() accepts slug and title, I don't think there is need for is_page( path ) or is_page_path function.

comment:5 kawauso3 years ago

function is_page_path( $path ) {
	$page = get_page_by_path( $path );
	return $page ? is_page( $page->ID ) : null;
}

Something like that?

-1 to having this in core, it seems like a niche and untidy way of doing things.

johnbillion3 years ago

comment:6 johnbillion3 years ago

  • Keywords has-patch added

Patch.

@hakre, @greuben and @kawauso: Why the opposition? is_page() accepts a slug, so accepting a full path is a natural extension. In fact it's not even a natural extension, I'd say it's expected behaviour. Multiple pages with different parents can have the same slug, so this eliminates the ambiguity.

comment:7 wonderboymusic9 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 3.7

Patch still applies cleanly to trunk - this patch makes sense to me, moving to 3.7 for discussion

comment:8 SergeyBiryukov8 months ago

Shouldn't get_page_by_path() be used instead?

comment:9 nacin8 months ago

Probably the best argument against this is that get_page_uri() triggers possibly unnecessary queries.

If it is to happen, it should be smarter. As in, explode by /, first check the slug, then check if it has a post_parent (and if not, then it's not a match if the path indicates it is a child page), then check the next post parent, etc. Essentially, it should walk its way up the tree one by one and compare, rather than a get_page_uri() which could cause a few queries and/or cache hits.

comment:10 helen7 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 3.7 to Awaiting Review

See nacin's comment for trying an alternate approach.

Jesper8007 months ago

comment:11 Jesper8007 months ago

I agree with Sergey Biryukov, get_page_by_path should be used for the approach suggested by Nacin. It runs a single query to fetch the ID, parent ID, slug and post type from the pages in the page path (exploded by /). After that, it checks the parts starting by the last child and moving up to the top ancestor.

However, when running this with many page paths in the array parameter passed to is_page, this might actually be slower than the get_page_uri approach.

I've patched this to use the get_page_by_path-approach (16802.2.patch).

If this is accepted, it should also be implemented for is_single.

comment:12 Jesper8007 months ago

  • Cc jeve0@… added
  • Keywords has-patch added; needs-patch removed

comment:13 wonderboymusic26 hours ago

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

comment:14 jnhghy12 hours ago

Why would

is_page($path)

be false when $path is actually a page?
I think that the function is_page() should answer with true if a page path is passed to it.
At least this makes sense to me.

Note: See TracTickets for help on using tickets.