Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#57039 closed defect (bug) (fixed)

WP 6.1 - get_page_by_title(null) returns a page

Reported by: stentibbing's profile stentibbing Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.1.1 Priority: normal
Severity: normal Version: 6.1
Component: Posts, Post Types Keywords: has-patch has-unit-tests fixed-major dev-reviewed
Focuses: Cc:

Description

Starting from version 6.1, get_page_by_title(NULL) returns a page instead of NULL.

Change History (18)

#1 @pbiron
2 years ago

  • Component changed from General to Posts, Post Types

@stentibbing thank you for the ticket and welcome to Trac!!

I can confirm that get_page_by_title( null ) in 6.1 indeed returns a WP_Post and that it did not in WP 6.0.x.

This bug was introduced in [54234].

Related: #36905

@spacedmonkey I think to easiest fix would be to check whether $page_title is empty before constructing the WP_Query and calling _doing_it_wrong() if it is and returning null in that case. Thoughts?

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

#2 @pbiron
2 years ago

@spacedmonkey on further testing, a _doing_it_wrong() when null (or the empty string) is passed is not the right thing to do.

Why? Because if there is a page (a post of the passed post type) which the empty string as it's post_title, then get_page_by_title( null ) in WP 6.0 would return it. So, my quick fix would be just another backwards incompatibility :-(

This ticket was mentioned in PR #3584 on WordPress/wordpress-develop by @spacedmonkey.


2 years ago
#3

  • Keywords has-patch has-unit-tests added

#4 @spacedmonkey
2 years ago

  • Milestone changed from Awaiting Review to 6.1.1

Added a PR #3584 and moved to 6.1.1 milestone.

#5 follow-up: @TimothyBlynJacobs
2 years ago

I don't think this patch fixes the issue because of @pbiron's second comment, right?

#6 in reply to: ↑ 5 ; follow-up: @pbiron
2 years ago

Replying to TimothyBlynJacobs:

I don't think this patch fixes the issue because of @pbiron's second comment, right?

Timothy: I think you're right.

#7 in reply to: ↑ 6 @stentibbing
2 years ago

Replying to pbiron:

Replying to TimothyBlynJacobs:

I don't think this patch fixes the issue because of @pbiron's second comment, right?

Timothy: I think you're right.

Semantically it would make sense if get_page_by_title("") would return posts with empty string as post_title and get_page_by_title(null) would return null.

#8 follow-up: @peterwilsoncc
2 years ago

Between this and #56991, let's revert the changes to get_page_by_title() and either redo or deprecate the function in 6.2.

#9 @peterwilsoncc
2 years ago

  • Owner set to peterwilsoncc
  • Status changed from new to assigned

#10 in reply to: ↑ 8 ; follow-up: @pbiron
2 years ago

Replying to peterwilsoncc:

Between this and #56991, let's revert the changes to get_page_by_title() and either redo or deprecate the function in 6.2.

I think reverting is probably the safe choice at this time. I like the caching that was added in #36905, but the backward incompatibility has caused problems on a number of my client sites.

#11 in reply to: ↑ 10 @spacedmonkey
2 years ago

Replying to pbiron:

Replying to peterwilsoncc:

Between this and #56991, let's revert the changes to get_page_by_title() and either redo or deprecate the function in 6.2.

I think reverting is probably the safe choice at this time. I like the caching that was added in #36905, but the backward incompatibility has caused problems on a number of my client sites.

I don’t think it is a good idea to revert this. It goes from be cached to not being cached. Which could result in performance problems that have relied on this function.

I don't see this bug as a big problem. This function was broken in pre 6.1, as it was unpredictable to begin with.

Leave as is and should deprecat this function in WordPress 6.2.

#12 @TimothyBlynJacobs
2 years ago

I strongly agree that this should be reverted. We now have two separate issues which both don't have a super straightforward fix. Given that 6.1.1 is being targeted for next week which will require a RC in the next few days, I don't think we should be doing another overhaul on this function in such a short period of time.

It goes from be cached to not being cached.

True. But it was uncached in 6.0. I think it's unlikely that many developers have adopted this API in the past week. It's still restricted by VIP for instance.

I don't see this bug as a big problem. This function was broken in pre 6.1, as it was unpredictable to begin with.

It's caused breakage for a number of users so far. It being unpredictable makes it a good candidate for deprecation in #57041. But if we are going to deprecate the function in the next release, then I don't think we should be committing an overhaul to the function right before.

#13 @spacedmonkey
2 years ago

If the function is deprecated in 6.2, I am not against reverting it here.

#15 @peterwilsoncc
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 54782:

Posts, Post Types: Revert get_page_by_title()'s use of WP_Query.

Revert to legacy database query in get_pages_by_title(). Due to the lack of orderby clause in the previous database query, it is not possible to gain consistent results by converting the function to a WP_Query wrapper.

Reverts [54271, 54242, 54234].

Props Bjorn2404, 10upsimon, dilipbheda, mukesh27, spacedmonkey, TimothyBlynJacobs, rjasdfiii, stentibbing, pbiron, pento.
Fixes #57039, #56991.
See #57041.

#16 @peterwilsoncc
2 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging in to the 6.1 branch.

#17 @TimothyBlynJacobs
2 years ago

  • Keywords dev-reviewed added

#18 @peterwilsoncc
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 54783:

Posts, Post Types: Revert get_page_by_title()'s use of WP_Query.

Revert to legacy database query in get_pages_by_title(). Due to the lack of orderby clause in the previous database query, it is not possible to gain consistent results by converting the function to a WP_Query wrapper.

Reverts [54271, 54242, 54234].

Props Bjorn2404, 10upsimon, dilipbheda, mukesh27, spacedmonkey, TimothyBlynJacobs, rjasdfiii, stentibbing, pbiron, pento.
Merges [54782] to the 6.1 branch.
Fixes #57039, #56991.
See #57041.

Note: See TracTickets for help on using tickets.