WordPress.org

Making WordPress.org

#2882 closed defect (fixed)

Invalid values in Devhub_Handbooks::add_query_vars() for query_vars

Reported by: grapplerulrich Owned by: obenland
Milestone: Priority: normal
Component: Handbooks Keywords: has-patch
Cc:

Description

The values for query_vars needs to be a STRING and INTEGER value.

This is currently not the case. https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-developer/inc/handbooks.php#L64

array(5) {
  ["is_handbook"] => bool(false)
  ["current_handbook"] =>bool(false)
  ["current_handbook_home_url"] =>bool(false)
  ["current_handbook_name"] => string(0) ""
  ["empty_post_type_search"] => bool(false)
}

Related #core38401

Attachments (2)

2882.patch (9.2 KB) - added by grapplerulrich 22 months ago.
2882.2.patch (9.9 KB) - added by grapplerulrich 20 months ago.

Download all attachments as: .zip

Change History (12)

#1 @grapplerulrich
22 months ago

  • Keywords has-patch added

The invalid values have been replaced with valid variables or switched to properties like done previously [4294]

#2 @johnbillion
22 months ago

2882.patch does not look correct to me. The query_vars filter expects the query variable names as values of the array. The keys are not relevant.

#3 @SergeyBiryukov
21 months ago

2882.patch seems correct to me.

[4294] fixed the issue for is_handbook_root, but a similar issue with empty_post_type_search was missed.

#4 follow-up: @johnbillion
21 months ago

2882.patch might fix the issue, but it's incorrect. The query vars passed through the query_vars filter is an array with query vars as the array element values, not query vars as the array element keys. It's filtering the WP::$public_query_vars property.

#5 in reply to: ↑ 4 ; follow-up: @SergeyBiryukov
21 months ago

Replying to johnbillion:

The query vars passed through the query_vars filter is an array with query vars as the array element values, not query vars as the array element keys.

Right, but query vars as array element keys is the current code, not what the patch suggests. The patch removes the invalid filter usage for empty_post_type_search, same as [4294] did for is_handbook_root. Why is that incorrect?

#6 in reply to: ↑ 5 @johnbillion
21 months ago

Replying to SergeyBiryukov:

Right, but query vars as array element keys is the current code, not what the patch suggests. The patch removes the invalid filter usage for empty_post_type_search, same as [4294] did for is_handbook_root. Why is that incorrect?

Yes you're correct, but the Devhub_Handbooks::add_query_vars() method still includes incorrect usage.

#7 @grapplerulrich
20 months ago

@johnbillion @SergeyBiryukov I have relooked at the documentation and fixed query_vars so that the names are as values and not keys.

I have tested the code locally..

The tests that I did were to check

#8 @ocean90
12 months ago

#3607 was marked as a duplicate.

#9 @obenland
10 months ago

@SergeyBiryukov Is this ready to go?

#10 @obenland
10 months ago

  • Owner set to obenland
  • Resolution set to fixed
  • Status changed from new to closed

In 7319:

Developer: Use query_vars correctly.

Props grapplerulrich, johnbillion, SergeyBiryukov.
Fixes #2882.

Note: See TracTickets for help on using tickets.