Skip to content
This repository was archived by the owner on Dec 14, 2021. It is now read-only.

Detect URL implementations that don't support search setter#143

Closed
JakeChampion wants to merge 2 commits intoinexorabletash:masterfrom
JakeChampion:safari-10-url
Closed

Detect URL implementations that don't support search setter#143
JakeChampion wants to merge 2 commits intoinexorabletash:masterfrom
JakeChampion:safari-10-url

Conversation

@JakeChampion
Copy link
Contributor

It looks like Safari 10.1 has an issue with the URL search setter. I've patched the feature detection in the polyfill to ensure that the polyfill is applied if URL implementations have a buggy search setter.
Here is a screenshot of the tests running on Safari 10.1:
Screenshot of Safari 10.1 running the url tests and having a test failure

Here is a screenshot of the tests running on Safari 10.1 after the feature detect was patched:
Screenshot of Safari 10.1 running the url test and having all the tests passing

@JakeChampion
Copy link
Contributor Author

@inexorabletash, would you be able to review this at all?
If you are busy with other projects and want some help maintaining this project, I'd be happy to help out as I use it quite a lot for https://polyfill.io

var url = new URL('http://example.com');
url.search = 'a=1&b=2';
if (url.href === 'http://example.com/?a=1&b=2') {
url.search = '';
Copy link

@Mouvedia Mouvedia Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a problem with this.
It detects https://bugs.webkit.org/show_bug.cgi?id=162345 which is pretty minor.
Replacing URL.prototype.search's setter would be better in that case.

@inexorabletash
Copy link
Owner

Sorry for the delay - definitely busy with other projects and not actively maintaining this; I appreciate the reviews by others.

There's also #151- if folks can agree on something that at least passes the same tests as now (and hopefully adds/passes more) I'll merge it.

@inexorabletash
Copy link
Owner

@JakeChampion - transferring ownership of this project is a possibility; I'm not even using it any more (I've gone for a subset).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants