Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick some more DOMURL changes #25142

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

nico
Copy link
Contributor

@nico nico commented Oct 19, 2024

bplaat and others added 8 commits October 19, 2024 10:11
(cherry picked from commit f2034270f14a72ebf0ac09493837c21bae0b4adf)
This is where I have been trying to put all of the URL tests.

(cherry picked from commit e2fb24c9b8e973542e8313ddb9e0f9f85a71c0e8)
(cherry picked from commit 5637dc43b2d6faf0576e7844581d0ced882d007f)
(cherry picked from commit 264b5160c2099f1aab6f06ece720de984ea994b2)
(cherry picked from commit 4bb211ba888b8fed9fd3da3163dc6e823d7e3c8a)
Made easier now that URL percent encode after encoding is also not
throwing any errors. This simplfies a bunch of error handling.

(cherry picked from commit df4739d7ced4159deb2b3e40ba6a1a08b7e7dd5b)
Quick sort is not a stable sort. This meant we had a subtle issue
implementing this portion of the spec comment:

 > The relative order between name-value pairs with equal names must
 > be preserved.

Switch to insertion sort which is a stable sort, and properly handles
keys which are the same.

Fixes 8 tests on  https://wpt.live/url/urlsearchparams-sort.any.html

(cherry picked from commit 1ba6dbd86c0a94e5f068e6586199866f7de6354e)
Because the type returned by to_string is a String, _not_ an
Optional<String>, the following code:

if (serialized_query.is_empty())
    serialized_query = {};

Was achieving nothing at all! Make sure that the type is an
Optional<String> so that we're not just setting an empty string to an
empty string.

(cherry picked from commit d755a83c09a5fba82a91f9cc3f9e3b2e07880127)
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 pr-needs-review PR needs review from a maintainer or community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants