add_query_arg function doesn't remove the empty '?' when an anchor exists

[benjamin.anakena]

To reproduce:

<?php
$url = 'https://domain.com/uri?param=value#anchor';
$url = add_query_arg( 'param', false, $url );
echo $url;

Expected value: https://domain.com/uri#anchor
Returned value: https://domain.com/uri?#anchor

[sabernhardt]

replace ?# with #

[sabernhardt]

@benjaminanakena Thanks for the report!

44499.patch​ works with the example.

[sabernhardt]

#52503 was marked as a duplicate. (remove_query_arg can create a similar URL)

[costdev]

Unit test added.

Applies patch 44499.1.diff.
Adds more test datasets.

Trac ticket: https://core.trac.wordpress.org/ticket/44499

[hellofromTonya]

Browser Test Report: same behavior before and after 44499.1.diff. No regression. Works before and after.

[hellofromTonya]

Test Report

Env:

• Browser: Chrome, Firefox, Edge, and Safari
• OS: macOS
• localhost: wp-env
• WordPress: trunk
• Theme: TT2
• Plugins: none

Steps

1. Go to "Posts" and then "Add New"
2. In the content, add some filler paragraphs.
3. And then add a header block towards the bottom of the content.
4. With the header block selected, click on "Advanced" and set the "HTML anchor" to anchor.
5. Set the permalinks to something other than "Plain".
6. View the new post in the front-end
7. Click in the browser's search bar and append ?param=value#anchor to the end of the URL. Press enter or return.

The post should reload and jump to the header block.

8. Apply the patch.
9. Repeat step 6 and 7.

Should again jump to the header block.

10. Repeat steps 6 to 7 using by appending this after the URL: ?#anchor.

Should again jump to the header block.

Results

There is no bug or issue in the front-end as the browser handles the query args and jumps to the anchor ✅

With and without the patch, each browser behaved the same ✅

[hellofromTonya]

While the add_query_arg() does not remove the unneeded ? when there are no query args, it works in each browser tested. Granted it seems odd to leave ? when it's not needed. But I wonder: Is there's a browser or device where this does not work?

For backend correctness, it makes sense to remove the unneeded ? character. This can help with further processing where that character is not expected and guard Core against changes in browsers or incompatibility between apps and browsers.

​PR 1902 is ready for commit.

[hellofromTonya]

Preparing commit.

[hellofromTonya]

In 52187:

HTTP API: Remove empty ? when only anchor remains in add_query_arg().

If after processing through add_query_arg() a ?# remains, this commit removes the unnecessary and unused ? character as there are no query args in the URL.

Includes tests.

Follow-up to [1823], [5193], [5999], [6005].

Props benjaminanakenam, sabernhardt, costdev, hellofromTonya.
Fixes #44499.

[hellofromTonya]

Thank you everyone for contributing! The patch has been committed and will ship in 5.9.

Committed via changeset https://core.trac.wordpress.org/changeset/52187.