Closed Bug 1626997 Opened 4 years ago Closed 3 years ago

Enable <link rel=preload> support by default

Categories

(Core :: Networking, task, P2)

Product:
Core
Component:
Networking
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Project Flags:
Webcompat Priority P2
Tracking Flags:
Tracking Status
firefox-esr78 --- wontfix
firefox85 --- fixed
firefox86 --- fixed

People

(Reporter: mayhemer, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, Whiteboard: [necko-triaged] [platform-rel-Wikipedia] )

Attachments

(1 file)

Bug 1626997 - Toggle the network.preload pref on on all channels. r=dragana
48 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
Details | Review

This just means to flip the network.preload preference to true.

Depends on stability, integrity and performance testing.

Priority: -- → P2

I wonder how prevalent this pattern is already:
<link rel="preload" as="style" onload="this.rel = 'stylesheet'" href="scripts/bootstrap/css/bootstrap.css">

See Also: → https://webcompat.com/issues/51644

(In reply to Mike Taylor [:miketaylr] from comment #1)

I wonder how prevalent this pattern is already:
<link rel="preload" as="style" onload="this.rel = 'stylesheet'" href="scripts/bootstrap/css/bootstrap.css">

With the patches for "preload as speculative load" this works. But it's hugely suboptimal because it's adding an unprioritized loop to the event queue that blocks the stylesheet from being applied and the engine to know there will be a style applied and optimize for it. Leaving rel at "stylesheet" is performing way better. Unless this has to be some kind of a lazy load; but that then defeats the purpose of preload which is about prioritization. It's like using express delivery and then leave the package for two days at your doorstep.

In other words: don't use this construct unless you really know what you are doing. I don't :)

See Also: → https://webcompat.com/issues/51983

Another top site using this pattern (ranks #85 in Turkey)

<link rel="preload" href="https://cdn.pr.trt.com.tr/static/font/fontawesome/css/all.min.css" as="style" onload="this.onload=null;this.rel='stylesheet'">

I see this on stackoverflow (but it does note that it won't work in Firefox, assuming people even read (or care...)): https://stackoverflow.com/a/46750893

Webcompat Priority: --- → P2

One more site with a similar pattern <link rel="preload" href="https://cdnjs.cloudflare.com/ajax/libs/magnific-popup.js/1.1.0/magnific-popup.css" as="style" onload="this.onload=null;this.rel='stylesheet'">

See Also: → https://webcompat.com/issues/52069

Found my way here trying to see why this WP plugin example page was looking broken in FF. The owner fixed it for now, but not sure if lazy loading still works with that fix in place.

See Also: → https://github.com/kevinweber/lazy-load-for-videos/issues/43

Hi devs!

My website does not look good in firefox either, because the new default functionality in firefox is not active, my website is https://www.convertidor.es thanks

(In reply to Jesús from comment #7)

Hi devs!

My website does not look good in firefox either, because the new default functionality in firefox is not active, my website is https://www.convertidor.es thanks

Why not just using <link rel="stylesheet">? Using <link rel="preload" as="style" onload="this.onload=null;this.rel='stylesheet'"> is not great, you should either have both the <link rel="preload"> and <link rel="stylesheet">, or just the later I guess. And you're putting <link rel="stylesheet"> in a <noscript> tag, so you can just remove the <noscript> basically.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

(In reply to Jesús from comment #7)

Hi devs!

My website does not look good in firefox either, because the new default functionality in firefox is not active, my website is https://www.convertidor.es thanks

Why not just using <link rel="stylesheet">? Using <link rel="preload" as="style" onload="this.onload=null;this.rel='stylesheet'"> is not great, you should either have both the <link rel="preload"> and <link rel="stylesheet">, or just the later I guess. And you're putting <link rel="stylesheet"> in a <noscript> tag, so you can just remove the <noscript> basically.

Thanks.

Because I think that is not good for Google.

Why not? It shouldn't cause any extra load.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Why not? It shouldn't cause any extra load.

As I see the feature is available but it is not enabled by default in Firefox because it is being verified and tested

This feature is good for Google.

(In reply to Jesús from comment #11)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Why not? It shouldn't cause any extra load.

As I see the feature is available but it is not enabled by default in Firefox because it is being verified and tested

This feature is good for Google.

I mean that instead of having:

    <link rel="preload" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" integrity="sha256-l85OmPOjvil/SOvVt3HnSSjzF1TUMyT9eV0c2BzEGzU=" crossorigin="anonymous" as="style" onload="this.onload=null;this.rel='stylesheet'"/>
    <noscript><link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" integrity="sha256-l85OmPOjvil/SOvVt3HnSSjzF1TUMyT9eV0c2BzEGzU=" crossorigin="anonymous" /></noscript>

You could have:

    <link rel="preload" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" integrity="sha256-l85OmPOjvil/SOvVt3HnSSjzF1TUMyT9eV0c2BzEGzU=" crossorigin="anonymous" as="style" />
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" integrity="sha256-l85OmPOjvil/SOvVt3HnSSjzF1TUMyT9eV0c2BzEGzU=" crossorigin="anonymous" />

And it'd work in all browsers, and get the benefit on <link rel="preload"> in Chrome as well (so no downside).

Ok thanks i will try

Thanks for the interest in this feature. But please bring these issues and discussions to web development forums instead of a polluting a mozilla bug with it. Thanks.

Sorry,

Where is a discussions forums to web development ?

Thanks.

(In reply to Jesús from comment #15)

Sorry,

Where is a discussions forums to web development ?

Thanks.

I think there is https://chat.mozilla.org/#/room/#front-end-web-development:mozilla.org where you should be able to discuss these issues.

Depends on: 1639607
Blocks: 1636087
Depends on: 1646776
Component: DOM: Core & HTML → Networking
Severity: normal → N/A
Whiteboard: [necko-triaged]

When will we can use <link rel="preload"> on Firefox?

The last experiment to confirm that we do not have any regressions is running. I hope it will went well and we will enable rel=preload in Firefox 84.

Hi. I noticed https://blog.mozilla.org/performance/2020/12/15/2020-year-in-review/ noting that rel-preload is shipped in Firefox 84. And I see network.preload set to true after updating to 84.
Does that mean it's shipped in release? If so, it's great and worth mentioning in developer notes as well.

As far as I can tell preload didn't make it to Firefox 84:

  • network.preload is false for me
  • I also don't observe preloads working in WebPageTest with Firefox 84
  • our RUM data for Fx83 and Fx84 show no change, and if preload was enabled, I'd expect big improvements

Having said that I see that Firefox Dev Edition 85 has the flag enabled.

Whiteboard: [necko-triaged] → [necko-triaged] [platform-rel-Wikipedia]

network.preload is true for me (and my customers...).

However, with a media tag of "print" it is causing an endless looping error for me. I am using the onerror attribute to try and append an alternate URL if the first fails to load. This is always getting triggered, and the onerror is looping when I append another link to the head. But it only happens with the media tag of "print"; screen and not defined works fine. Onload also is never called.

I've been able to work around this by changing my JS to only add another link tag if one doesn't already exist with that URL. But it sucks that I had to do that.

So... if I'm an idiot and did something wrong, please let me know. But it's been working for a while and only quit working upon upgrading recently. I know 84.0.2 causes this problem, maybe 84.0.1, too, not sure.

<link href="https://sparqlogon.azureedge.net/SharedContent/styles/Print.1.1.css" onerror="this.onerror=null;window.useBackupCssUrl('https://logon.sparqdata.com/SharedContent/styles/Print.1.1.css')" as="style" onload="this.onload=null;this.rel='stylesheet'" media="print" rel="preload" type="text/css">

Can you attach a complete test-case which doesn't work in a new bug?

Flags: needinfo?(jesse.sierks)

Afaict we've shipped this code via normandy or what not, so let's do
this.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e205e7b4a73c
Toggle the network.preload pref on on all channels. r=dragana

Eric, AIUI the normandy recipe should only be enabled for 84. Should this patch be uplifted to beta 85 so that we don't ship, then unship, then ship the feature?

Flags: needinfo?(esmyth)

Emilio, yes, the recipe is currently set to enable only for 84. Two options to avoid unshipping: 1) uplift the patch to 85, or 2) update the recipe to include 85 and let the patch ride the train.

Flags: needinfo?(esmyth)

Comment on attachment 9196197 [details]
Bug 1626997 - Toggle the network.preload pref on on all channels. r=dragana

Beta/Release Uplift Approval Request

  • User impact if declined: We'd unship preload in 85 which seems unfortunate.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): We've shipped this pref flip in 84 via normandy.
  • String changes made/needed: none
Attachment #9196197 - Flags: approval-mozilla-beta?

https://hg.mozilla.org/mozilla-central/rev/e205e7b4a73c

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

the rollout (https://normandy.cdn.mozilla.net/api/v3/recipe/1102/) is currently at 50%. Are we happy for that to go 100% with the 85 release?

Flags: needinfo?(jstutte)

I think that's ok, thanks.

Flags: needinfo?(jstutte) → needinfo?(jcristau)

Comment on attachment 9196197 [details]
Bug 1626997 - Toggle the network.preload pref on on all channels. r=dragana

approved for 85.0b9

Flags: needinfo?(jcristau)
Attachment #9196197 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

FYI this feature got documented in FF85 as Web technology for developers > HTML: HyperText Markup Language > Preloading content with rel="preload". Can you please provide advice about the section/example on including a mime type.

The example in that section specifies two links to preload sintel-short.mp4 and sintel-short.mp4. Later on in the body the <video controls> provides both files as sources. Without preloading the page will just download the first type that it supports. I am wondering how preloading affect this ...

Does it

  • preload all items that are specified irrespective of the video controls
  • preload the first item with a particular name prefix it can handle - ie if both files are named "sintel-short" it just downloads the first one?
  • preload all items that it can handle irrespective of name
  • preload those files can't handle/display

Ultimately I want to confirm if this statement after the example code makes sense:

So in this case, browsers that support MP4s will preload and use the MP4, making the video player hopefully smoother/more responsive for users. Browsers that don't support MP4 can still load the WebM version, but don't get the advantages of preloading. This shows how preloading content can be combined with the philosophy of progressive enhancement.

Flags: needinfo?(emilio)

I think that statement is wrong. If the browser supports both webm and mp4 it would preload both, see the code here.

I think the <link rel="preload" type="video/webm"> should be removed from the example, then the example and the text both would be correct (you'd only preload the mp4 if you support it, which would be the primary source of the video, but if the browser doesn't support it you still get the webm).

Flags: needinfo?(emilio)

Don't know what's wrong, but this code below doesn't work in version 85 (the style is ignored completely). Was it supposed to be fixed?

<link rel="preload" href="style.css" as="style">

That just preloads the style, but the style isn't used anywhere. You need to add a <link rel="stylesheet" href="style.css"> there as well, or onload="this.onload = null; this.rel = 'stylesheet'" to the <link rel="preload">.

Right.. it's going to take many questions from users why the code I posted doesn't work, but your answer helped, thank you.

BTW - I tried and this code below seems to work (ie. it loads the style), but doesn't actually preloads the style?

<link rel="preload stylesheet" href="style.css" as="style">

That seems like it should work, why do you say it's not getting preloaded? We do get to HTMLLinkElement::StartPreload on that test-case, fwiw.

Note that if you're not testing from an http/https site it won't work on 85 because of bug 1685830.

Yes, it works, just wanted to make sure it's going to work in the future too (I like it because it's a short version). Thank you.

Yeah, that is indeed nicer. Note that there's some weirdness since that should get two load events (one from the stylesheet, one from the preload), which might be a bit odd. But that is also true of the <link rel=preload as=style onload=...> approach so... Plus yours works without JS which is nicer.

Forgive me if I missed it but is this fix going to trickle back to esr firefox see:
https://github.com/webcompat/web-bugs/issues/67273
I've also experienced other random problems which, after reading through this bug were most likely caused by "network.preload" being set "False"

Flags: needinfo?(emilio)

I'd say it's unlikely for a patch enabling features like this to be backported to esr... Though it's not really my call I guess, and if the problems it causes are really severe it might make sense... Ryan / Julien, do you have opinions?

Flags: needinfo?(ryanvm)
Flags: needinfo?(jcristau)
Flags: needinfo?(emilio)

Feature backports are out of scope for ESR releases, yes.

status-firefox-esr78: --- → wontfix
Flags: needinfo?(ryanvm)
Flags: needinfo?(jcristau)
You need to log in before you can comment on or make changes to this bug.