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

URL.parse() static method docs #33207

Merged
merged 14 commits into from
May 7, 2024
Merged

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Apr 22, 2024

FF126 supports the URL.parse static method. This is just like the URL constructor except that it doesn't throw if the parameter strings define an invalid URL.

This adds a page with a live example, and does some fixes and cross linking to the URL constructor.

Related docs work can be tracked in #33080

@hamishwillee hamishwillee requested a review from a team as a code owner April 22, 2024 07:54
@hamishwillee hamishwillee requested review from wbamberg and removed request for a team April 22, 2024 07:54
@github-actions github-actions bot added the Content:WebAPI Web API docs label Apr 22, 2024
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Apr 22, 2024
Copy link
Contributor

github-actions bot commented Apr 22, 2024

Preview URLs

External URLs (2)

URL: /en-US/docs/Web/API/URL_API/Resolving_relative_references
Title: Resolving relative references to a URL


URL: /en-US/docs/Web/API/URL/parse_static
Title: URL: parse() static method

(comment last updated: 2024-05-07 00:03:38)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

I guess the big comment here which is probably out of scope for this PR is that we should have a definitive separate guide to "resolving relative URLs" that we can point to from all the places that do this.

files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/url/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/url/index.md Outdated Show resolved Hide resolved
@hamishwillee hamishwillee requested a review from a team as a code owner April 29, 2024 06:01
@hamishwillee hamishwillee requested review from pepelsbey and removed request for a team April 29, 2024 06:01
@hamishwillee
Copy link
Collaborator Author

@wbamberg All ready for another review. Includes new doc for Resolving URLs.

@wbamberg
Copy link
Collaborator

wbamberg commented May 4, 2024

Thank you for writing this! I think relative URL resolution is quite an important thing that AFAIK is documented nowhere on MDN. The nearest thing is https://developer.mozilla.org/en-US/docs/Learn/Common_questions/Web_mechanics/What_is_a_URL which doesn't do into enough detail.

But I have a couple of high level comments on this.

First, I think it's better to present this as a generic algorithm - "how browsers handle relative URLs" - rather than specifically as a feature of the URL interface. Because it is an algorithm for how browsers handle relative paths in general, and URL is just referring to that.

Second, I like that you have lots of examples and I appreciate explaining it in a fairly informal way. But it would be great if we could also present it in a more rigorous/precise way. The trick is to explain https://datatracker.ietf.org/doc/html/rfc3986.html#section-5.2 in a way that is more comprehensible but just as precise.

I've had a shot at writing this up below, perhaps we could work out some version of this? What do you think?


We could present URL resolution as a three-step process:

  • split both URLs into their components
  • calculate the value of each combined component
  • compose the components

Split URLs

First, split base and relative URLs into their components (we already define the components in https://developer.mozilla.org/en-US/docs/Learn/Common_questions/Web_mechanics/What_is_a_URL):

  • scheme
  • authority
  • path
  • query
  • fragment

Calculate component values

  • if relative.scheme exists, components are:

    • relative.scheme, relative.authority, remove_dots(relative.path), relative.query, relative.fragment
  • if relative.authority exists, components are:

    • base.scheme, relative.authority, remove_dots(relative.path), relative.query. relative.fragment
  • if relative.path is "" and !relative.query, components are:

    • base.scheme, base.authority, base.path, base.query. relative.fragment
  • if relative.path is "" and relative.query, components are:

    • base.scheme, base.authority, base.path, relative.query. relative.fragment
  • if relative.path starts with "/", components are:

    • base.scheme, base.authority, remove_dots(relative.path), relative.query. relative.fragment
  • if relative.path does not start with "/", components are:

    • base.scheme, base.authority, remove_dots(merge(base.path, relative.path)), relative.query. relative.fragment

Merging base and relative paths

  • if base has an authority and an empty path, then path is "/" followed by relative.path
  • otherwise, remove from base.path all characters after the rightmost "/", (or the whole of base.path if base.path does not contain "/"), then append relative.path

(I'm not sure if we can assume that base must have an authority, and therefore simplify this a bit?)

Remove dots

This is the process of removing "./" characters and processing "../" to remove parents. This is quite hard to explain precisely...

Compose components

Finally we combine the components to produce a single URL:

  • if scheme: add scheme + ":"
  • if authority: add "//" + authority
  • add path
  • if query: add "?" + query
  • if fragment: add "#" + fragment

Again, can we assume that scheme and authority are required for HTTPS URLs, so as to simplify the explanation?

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented May 6, 2024

@wbamberg

  1. Yes, I think this approach has merit. In particular because it catches all the edge cases - including a few like what happens if the relative URL is "", which I was aware of from testing but glossed over. I also prefer it because it is scalable in the event that the algorithm changes in some way.
  2. Should it still live under the API doc where I have put it? Given the URL API is effectively a standardized implementation of that algorithm it probably makes sense, and I can't think of a better place.

My concern here is that I want to get docs for URL.parse() in to meet my FF126 release goals, and I have many other tasks to get through before that happens next week. This already took longer than expected and I think having some docs on the other things is more important than getting this perfect.

So if we agree this is the right place for the doc and that the doc is "better than what was there before", then I suggest I rename to "how browsers handle relative URLs" and you merge as is. Then I come back to this after the release (or you can hack at it given you clearly have a better idea of this than me)

@wbamberg
Copy link
Collaborator

wbamberg commented May 6, 2024

@wbamberg

  1. Yes, I think this approach has merit. In particular because it catches all the edge cases - including a few like what happens if the relative URL is "", which I was aware of from testing but glossed over. I also prefer it because it is scalable in the event that the algorithm changes in some way.

Yes, there are a number of other cases too, like a relative URL of "//example.org", which is scheme-relative, or a relative URL of "this/../that" which eats its own parent, not the parent of the base, or URLs with queries and fragments.

  1. Should it still live under the API doc where I have put it? Given the URL API is effectively a standardized implementation of that algorithm it probably makes sense, and I can't think of a better place.

I did think about this, but the preamble for the URL API says "The URL API is a component of the URL standard, which defines what constitutes a valid Uniform Resource Locator and the API that accesses and manipulates URLs." which makes it seem like a good enough place. And there really isn't anywhere else except Learn, and I think that is wrong (the algorithm is not learning content).

My concern here is that I want to get docs for URL.parse() in to meet my FF126 release goals, and I have many other tasks to get through before that happens next week. This already took longer than expected and I think having some docs on the other things is more important than getting this perfect.

So if we agree this is the right place for the doc and that the doc is "better than what was there before", then I suggest I rename to "how browsers handle relative URLs" and you merge as is. Then I come back to this after the release (or you can hack at it given you clearly have a better idea of this than me)

That's fair. I wasn't actually expecting you to add this guide at all, as it is fairly tangential to your original PR.

(or you can hack at it given you clearly have a better idea of this than me)

I don't, really, I just spent a bit of time trying to understand the RFC. But there is definitely still work to do to make sure it is correct and complete.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks!

TBH I didn't put a lot of time into reviewing the guide, since if it will get rewritten anyway there didn't seem like much point.

files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/url/parse_static/index.md Outdated Show resolved Hide resolved
@wbamberg
Copy link
Collaborator

wbamberg commented May 6, 2024

It seems like the spec deprecates the whole "relative URL" terminology:

NOTE: Previous specifications used the terms "partial URI" and
"relative URI" to denote a relative reference to a URI. As some
readers misunderstood those terms to mean that relative URIs are a
subset of URIs rather than a method of referencing URIs, this
specification simply refers to them as relative references.

(https://datatracker.ietf.org/doc/html/rfc3986.html#section-1.2.3)

@hamishwillee
Copy link
Collaborator Author

Thanks @wbamberg

I have

  • accepted your suggestions
  • renamed the document to "Resolving relative references to a URL" and cross linked around the place
  • In the three affected docs I now refer to relative URLs as relative references as per spec.
  • Created Resolving relative references - improve topic #33455 to keep the follow on fixes alive.

@hamishwillee hamishwillee requested a review from wbamberg May 7, 2024 00:16
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍 thank you @hamishwillee !

@wbamberg wbamberg merged commit d7342e4 into mdn:main May 7, 2024
9 checks passed
dalps pushed a commit to dalps/content that referenced this pull request May 8, 2024
* URL.parse() static method docs

* Apply suggestions from code review - easy ones

Co-authored-by: wbamberg <[email protected]>

* Fix up url and following note to be less repetitive

* Remove example description from log

* Resolved, not appended

* Base URL resolution is more complicated than appending

* Update files/en-us/web/api/url/parse_static/index.md

Co-authored-by: Denis Pushkarev <[email protected]>

* Add resolving relative URLs

* Correct parse/url const

* Update resolving relative URLs

* Remove some no longer needed examples

* Apply suggestions from code review

Co-authored-by: wbamberg <[email protected]>

* Apply suggestions from code review

* Move resolving topic and refer to relative references

---------

Co-authored-by: wbamberg <[email protected]>
Co-authored-by: Denis Pushkarev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants