Skip to content

Allow relative URLs and http(s) scheme#37

Closed
devsnek wants to merge 1 commit intowhatwg:mainfrom
devsnek:better
Closed

Allow relative URLs and http(s) scheme#37
devsnek wants to merge 1 commit intowhatwg:mainfrom
devsnek:better

Conversation

@devsnek
Copy link
Copy Markdown
Contributor

@devsnek devsnek commented Oct 26, 2022

Addresses #20.

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

@ricea
Copy link
Copy Markdown
Collaborator

ricea commented Jan 20, 2023

The PR is fine, but Google is neutral on the feature. We won't support unless other browsers ship it first.

@lucacasonato
Copy link
Copy Markdown
Member

Deno would be in favor of landing this. Allowing only ws: and wss: protocols is a constant source of pain for web devs. A very simple query on GitHub Code Search reveals at least 9k candidate files in open source that have if (protocol == "https:") scheme = "wss:".

Folks, please let's ship this. It's trivial, it has tests, and it's going to make everyone's lives just slightly less terrible.

So, I'll ask for positions from WebKit and Mozilla here again: @annevk, @saschanaz, thoughts?

Comment on lines +132 to +135
1. If |requestURL|'s <a for=url>scheme</a> is "`ws`", set |requestURL|'s <a for=url>scheme</a>
to "`http`".
1. Otherwise, if |requestURL|'s <a for=url>scheme</a> is "`wss`", set |requestURL|'s
<a for=url>scheme</a> to "`https`".
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's do this normalization upfront in the constructor so it doesn't leak out in a number of places.

annevk pushed a commit that referenced this pull request May 11, 2023
And thereby relative URLs. They are instantly normalized to the ws: and wss: schemes.

Tests: web-platform-tests/wpt#39955.

Closes #37.
@annevk
Copy link
Copy Markdown
Member

annevk commented May 11, 2023

Since there was no follow-up, I decided to move forward with this in #45 with a slightly different approach. Hope that's okay.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants