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.

Sign up for GitHub

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

Jump to bottom

fetchPriority enabled by default in Firefox 132 #24518

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

fetchPriority enabled by default in Firefox 132 #24518

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9632404
fetchPriority enabled by default in Firefox 132
acreskeyMoz Sep 25, 2024
4aa7eba
Request.init_priority_parameter
hamishwillee Oct 1, 2024
3105e91
HTTP 103 status - add fetchpriority
hamishwillee Oct 1, 2024
dd5a9e8
HTTP 103 - add the tag for priority
hamishwillee Oct 1, 2024
198e1cc
LINK header mirror link element
hamishwillee Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions api/HTMLImageElement.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,14 +481,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
9 changes: 1 addition & 8 deletions api/HTMLLinkElement.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
9 changes: 1 addition & 8 deletions api/HTMLScriptElement.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
2 changes: 1 addition & 1 deletion api/Request.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@
},
"edge": "mirror",
"firefox": {
"version_added": false
"version_added": "132"
},
"firefox_android": "mirror",
"ie": {
Expand Down
9 changes: 1 addition & 8 deletions html/elements/img.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
9 changes: 1 addition & 8 deletions html/elements/link.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
9 changes: 1 addition & 8 deletions html/elements/script.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,7 @@
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "127",
"flags": [
{
"type": "preference",
"name": "network.fetchpriority.enabled",
"value_to_set": "true"
}
],
"version_added": "132",
"impl_url": "https://bugzil.la/1797715"
},
"firefox_android": "mirror",
Expand Down
33 changes: 33 additions & 0 deletions http/status.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,39 @@
"deprecated": false
}
},
"fetchpriority": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fred-wang This adds fetchpriority subfeature to 103 status. The values mirror those for the <link> feature - though for chrome this header wasn't implemented until 103, so I used that instead of 101 (which was the value used in link). Reasonable?

My understanding is that this header may be sent by a server prior to returning the final resource to indicate resources that the browser is likely going to want to request early. So basically this might be used to tell the browser to start loading a resource at higher priority before it parses the resource and finds the <link> tag that suggests it should do so - right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure whether/when chrome or webkit implemented the 103 header, but yeah that seems reasonable.

I think your understanding is correct, but actually this is also true for e.g. 200 OK. The subtility here is that Early Hints can be sent before the final response has arrived, allowing resources to be loaded even earlier.

Copy link
Contributor

Choose a reason for hiding this comment

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

Chrome implemented 103 preloads (Safari only supports preconnects in 103 now, where fetchpriority is not relevant so can probably be ignored for now).

It appears from the WPT's that you added that Chrome doens't error on fetchpriority on 103s:
https://wpt.fyi/results/loading/early-hints/preload-fetchpriority.h2.window.html?label=experimental&label=master&aligned
But we don't think it does anything special with it.

To us in Chrome it doesn't really make sense here as 1) high priority requests will be requested right anyway as soon as these are seen in headers and 2) low priority requests really shouldn't be sent in 103 requests. Not sure if Firefox was thinking of specific use cases for this that I'm not seeing? Or if it was just easier to support it everywhere rather than carve out an exception for link headers?

So you can argue it's "supported" in Chrome in that in that it won't error, but not sure if it's more confusing than useful to add in here?

CC: @pmeenan

Copy link
Collaborator

@hamishwillee hamishwillee Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@fred-wang Can you comment on the point immediately above ^^^: Not sure if Firefox was thinking of specific use cases for this that I'm not seeing? Or if it was just easier to support it everywhere rather than carve out an exception for link headers?.

@tunetheweb My assumption here was that if a browser sees a Link header it would download it, but the priority would still be affected by settings like preload and fetchpriority. In otherwords, if you have a lot of stuff to download, the additional prioritisation hint is still useful. I'm pretty ignorant of this stuff, so hopefully @fred-wang can clarify what he things support means for a browser.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thinks supporting fetchpriority in 103 came after a review by @mb and I believed it turned out that implementing the generic support for Link header implied support for 103 too. In https://bugzilla.mozilla.org/show_bug.cgi?id=1882084 in addition to the basic test mentioned we also check that fetchpriority affects our internal priority in 103 header. But I don't think we had any specific use case in mind besides supporting what's the spec says. So I agree, probably documenting support for the generic Link header is the right thing to do...

"__compat": {
"support": {
"chrome": {
"version_added": "103"
},
"chrome_android": "mirror",
"edge": "mirror",
"firefox": {
"version_added": "132"
},
"firefox_android": "mirror",
"ie": {
"version_added": false
},
"oculus": "mirror",
"opera": "mirror",
"opera_android": "mirror",
"safari": {
"version_added": "17.2"
},
"safari_ios": "mirror",
"samsunginternet_android": "mirror",
"webview_android": "mirror",
"webview_ios": "mirror"
},
"status": {
"experimental": false,
"standard_track": true,
"deprecated": false
}
}
},
"preconnect": {
"__compat": {
"description": "<code>rel=preconnect</code>",
Expand Down