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

FF132 fetchpriority docs updates #36142

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Oct 1, 2024

This updates the docs around the fetchpriority attribute/property and related headers. Most of this is tidying up docs to current standard format - i.e. it is churn.

Related docs work can be tracked in #36121

@github-actions github-actions bot added Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed labels Oct 1, 2024
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Preview URLs (10 pages)
External URLs (5)

URL: /en-US/docs/Web/API/HTMLImageElement/fetchPriority
Title: HTMLImageElement: fetchPriority property


URL: /en-US/docs/Web/API/Request/Request
Title: Request: Request() constructor


URL: /en-US/docs/Web/API/HTMLLinkElement/fetchPriority
Title: HTMLLinkElement: fetchPriority property


URL: /en-US/docs/Web/API/HTMLScriptElement/fetchPriority
Title: HTMLScriptElement: fetchPriority property


URL: /en-US/docs/Web/HTTP/Headers/Link
Title: Link

(comment last updated: 2024-10-11 05:08:00)

@fred-wang
Copy link
Contributor

@hamishwillee Per the discussion on the other thread, it's probably not worth highlighting the 103 early hint specifically, but just that we support fetchpriority in Link HTTP headers in general. (unless @mb says the contrary).

Also I guess another thing is devtools, but I'm not super familiar with that. See https://bugzilla.mozilla.org/show_bug.cgi?id=1584663#c5

@acreskeyMoz can probably help on the last item regarding performance optimization.

@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Oct 4, 2024
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs labels Oct 7, 2024
@hamishwillee hamishwillee marked this pull request as ready for review October 8, 2024 04:08
@hamishwillee hamishwillee requested review from a team as code owners October 8, 2024 04:08
@hamishwillee hamishwillee requested review from Elchi3, estelle, fred-wang and pmeenan and removed request for a team October 8, 2024 04:08
@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Oct 8, 2024

@fred-wang @pmeenan This is ready for a review if you are interested. Despite the churn, most of the change is cosmetic, related to moving to current MDN norms for format, and improved cross linking. For example, most of the docs now link to the chrome optimisation doc that mentions this.

It is perhaps a little more more clear about the internal priority and the set priority being relative to other files of the same type, and being browser dependent. There is also an example in the HTTP Link header topic that covers this case.

Copy link
Contributor

@pmeenan pmeenan left a comment

Choose a reason for hiding this comment

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

LGTM.

It still feels a bit off on the language because, at least in Chrome, it is an input that is taken into consideration when creating the internal priority so it doesn't directly compare against other internal priorities but that doesn't feel like a developer-facing distinction and as-written should have the same net result.

Thanks for cleaning it up.

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@hamishwillee mostly a bunch of nitpicky stuff, not much needs updating really. Good work dude.

{{domxref("HTMLImageElement")}} interface represents a hint given to the browser on how
it should prioritize the fetch of the image relative to other images.
The **`fetchPriority`** property of the {{domxref("HTMLImageElement")}} interface represents a hint to the browser indicating how it should prioritize fetching a particular image relative to other images.
It reflects the [`fetchpriority`](/en-US/docs/Web/HTML/Element/img#fetchpriority) attribute of the corresponding {{htmlelement("img")}} element, for the allowed values.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "for the allowed values" mean? Are there only a certain subset of attribute values that the property can be used to set? Phrasing seemed odd and made me question this. If it reflects it, pure and simple, remove this?

Copy link
Contributor

Choose a reason for hiding this comment

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

SAme comment on the link and script pages, too.


The property allows a developer to signal that fetching a particular image early in the loading process has more or less impact on user experience than a browser can reasonably infer when assigning an internal priority.
This in turn allows the browser to increase or decrease the priority, and potentially load the image earlier or later than it would otherwise.
The property should be used sparingly, as excessive or incorrect prioritization can degrade performance.
Copy link
Contributor

Choose a reason for hiding this comment

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

This all makes sense, but it feels to me like the page needs a short paragraph to describe some actual usage cases of the property (same for the link and script versions). And how does it work alongside other loading performance-related features such as preload and lazy load.

I appreciate this is discussed in great depth in the linked web.dev article, but it feels like this page could benefit with a little bit of usage description.

Use it sparingly for exceptional cases where the browser may not be able to
infer the best way to load the image automatically. Over use can result in
degrading performance.
- : No user preference for the fetch priority.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- : No user preference for the fetch priority.
- : Don't set a user preference for the fetch priority.

Feels like this needed to be changed to make it more of a complete sentence, and more in keeping with the instructive style of the other definitions. Same for the link and script pages too.

</tbody>
</table>
- `TypeError`
- The URL has credentials, such as `http://user:[email protected]` or cannot be parsed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't rendering correctly; you're missing the colon.

- `auto`
- : Default: Signals automatic determination of fetch priority relative to other images.
- : No user preference for the fetch priority.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, feels like this should be written more constructively, to match the style of the other value descriptions. Same for the other HTML attribute descriptions.

@@ -9,7 +9,8 @@ browser-compat: http.headers.Link

The HTTP **`Link`** [entity header](/en-US/docs/Glossary/Entity_header) field provides a means for serializing one or more links in HTTP headers. This header has the same semantics as the HTML {{HTMLElement("link")}} element. The benefit of using the `Link` header is that the browser can start preconnecting or preloading resources before the HTML itself is fetched and processed.

In practice, most [link types](/en-US/docs/Web/HTML/Attributes/rel) don't have an effect in the HTTP header. For example, the `icon` relation only works in HTML, and `stylesheet` does not work reliably across browsers (only in Firefox). The only relations that work reliably are `preconnect` and `preload`, which can be combined with {{HTTPStatus(103, "103 Early Hints")}}.
In practice, most [link types](/en-US/docs/Web/HTML/Attributes/rel) don't have an effect in the HTTP header. For example, the `icon` relation only works in HTML, and `stylesheet` does not work reliably across browsers (only in Firefox).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In practice, most [link types](/en-US/docs/Web/HTML/Attributes/rel) don't have an effect in the HTTP header. For example, the `icon` relation only works in HTML, and `stylesheet` does not work reliably across browsers (only in Firefox).
In practice, most [link types](/en-US/docs/Web/HTML/Attributes/rel) don't have an effect on the HTTP header. For example, the `icon` relation only works in HTML, and `stylesheet` does not work reliably across browsers (only in Firefox).

Copy link
Contributor

Choose a reason for hiding this comment

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

Or am I wrong? Feels like a weird phrasing when you first read it. But now I'm not so sure.

Even when using [`preload`](/en-US/docs/Web/HTML/Attributes/rel/preload) to fetch a resource as early as possible, different types of content will be fetched earlier or later based on the browser's internal prioritization.
The [`fetchpriority`](/en-US/docs/Web/HTML/Element/link#fetchpriority) attribute can be used to hint to the browser that a particular resource will have a greater or lesser relative impact on user experience than other resources of the same type.

For example, the header below might be used to preload `style.css` at higher priority than other stylesheets:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For example, the header below might be used to preload `style.css` at higher priority than other stylesheets:
For example, the header below might be used to preload `style.css` with a higher priority than other stylesheets:

Link: </style.css>; rel=preload; as=style; fetchpriority="high"
```

Note that both the internal prioritization for fetching resources and the effect of the `fetchpriority` are browser dependent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note that both the internal prioritization for fetching resources and the effect of the `fetchpriority` are browser dependent.
Note that both the internal prioritization for fetching resources and the effect of the `fetchpriority` parameter are browser-dependent.

```

Note that both the internal prioritization for fetching resources and the effect of the `fetchpriority` are browser dependent.
The `fetchpriority` attribute should be used sparingly, and only in cases where a browser would be unable to infer that a particular resource should be treated with a different priority.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `fetchpriority` attribute should be used sparingly, and only in cases where a browser would be unable to infer that a particular resource should be treated with a different priority.
The `fetchpriority` parameter should be used sparingly, and only in cases where a browser cannot infer that a particular resource should be treated with a different priority.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the context of the HTTP header, would it be a parameter? "Parameters" is used earlier on the page. Or in HTTP terms, would "directive" be more accurate? In any case, "attribute" probably isn't right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs 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.

4 participants