Closed Bug 1847811 Opened 1 year ago Closed 1 year ago

Preload links to not fire an error event if network.preload is disabled

Categories

(Core :: DOM: Networking, defect, P2)

Firefox 116
defect

Tracking

()

RESOLVED FIXED
118 Branch
Tracking Status
firefox118 --- fixed

People

(Reporter: matt, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [necko-triaged], [wptsync upstream])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36

Steps to reproduce:

  • Disable the network.preload config in Firefox
  • Then load the following index.html page
<!DOCTYPE html>
<html>
  <head></head>
  <body>
    <script>
      let link = document.createElement("link");
      link.setAttribute("rel", "preload");
      link.setAttribute("as", "style");
      link.setAttribute("href", "404.css");
      let id = setTimeout(() => {
        console.log("timed out");
      }, 3000);
      link.onload = () => {
        clearTimeout(id);
        console.log("onload");
      };
      link.onerror = () => {
        clearTimeout(id);
        console.log("onerror");
      };
      console.log(
        'appending <link rel="preload" as="style" href="404.css"> to <head>'
      );
      document.head.appendChild(link);
    </script>
  </body>
</html>

Actual results:

No onload or onerror is fired for the <link> element

Expected results:

I would have expected link.onerror to fire if preloading is disabled, since the requested link failed to preload. This gives application/framework developers a way to detect if the preload worked (or failed) and act accordingly.

Without either firing - is there any other way to detect that the link preload will not work short of an arbitrary timeout?

The Remix JS framework currently relies on link preloads to preload stylesheets on client-side/soft navigations to avoid a FOUC on the destination page - but when the preload link never resolves (either via onload or onerror) we end up stuck waiting on one of them.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Component: DOM: Core & HTML → DOM: Networking

I think network.preload mostly controls whether we parse the rel=preload part.
So otherwise it's just treated as an unknown rel value, so we don't attempt to preload it - not that preloads are blocked.

Matt, could you expand on why that would be a good idea? Who is this affecting - AFAICT the pref is true by default.

Flags: needinfo?(matt)

So when this setting is disabled, it sounds like it's functionally the same as inserting <link as="style" href="404.css"> into the DOM - which would not emit load/error events?

I guess my question is - is there any way to programmatically detect that <link rel="preload" as="style" href="404.css" /> will never emit the load or error event when inserted in the DOM?

The Remix web framework currently tries to leverage the web platform where possible and is using <link rel="preload"> links to preload stylesheets for destination routes so there is no FOUC.

So assume the HTML at /a looks like this:

<link rel="stylesheet" href="a.css" />
<h1>Page A</h1>

When performing a client-side/soft navigation via history.pushState(null, null, '/a'), the <link> stylesheet is not render blocking (as it is on an initial document load). So the heading will render initially without styles applied (while the stylesheet loads) and then only when the stylesheet loads will any h1 styles apply. This causes a flash of unstyled content which depends on the latency of the users connection.

To avoid this, Remix leverages link preloads to prime the cache with the stylesheet so that when page /a is updated in the DOM, the styles apply immediately because the <link rel="stylesheet"> tag can be served from the preload cache.

The pseudocode for a client side navigation is effectively:

function preloadStylesheet(href) {
  return new Promise((resolve) => {
    let link = document.createElement("link");
    link.setAttribute("rel", "preload");
    link.setAttribute("as", "style");
    link.setAttribute("href", href);
    link.onload = () => resolve();
    link.onerror = () => resolve();
    document.head.appendChild(link);
  });
}

// Called when <a href="/a"> is clicked
async function onClick() {
  e.preventDefault();
  await preloadStylesheet("a.css");   // prime the cache (ignoring errors)
  history.pushState(null, null, e.target.href) // update the URL
  updateDom("/a") // render the contents for /a
}

But the problem we're running into is that if a user has this setting disabled, then preloadStylesheet will never resolve and Remix will never render the destination route. I think a users expectation when disabling this flag is that their browsing experience may be slower, but not broken.

So the question sort of boils down to:

  • Is there any way for Remix to know that this preference is enabled and/or that this preload link is useless and will never emit a load/error event? If we could determine this, we could skip the preloads and accept the FOUC.
  • Or, is this approach of priming the cache fundamentally incompatible with the network.preload:false behavior?

You can see this bug in action if you disable the preference, go to https://remix.run/, and click the Docs link in the upper right hand corner. /docs has stylesheets associated with it - and they never preload so we never route you to the docs page.

It's not ideal, but to work around this issue we've added some code to timeout these preload attempts after a few seconds and eventually disable the preload functionality entirely if enough timeouts are encountered in a session. This allows users with this setting disabled to still navigate even if it's slower due to the timeouts.

Flags: needinfo?(matt)

Maybe Emilio has some thoughts here.

I'm also curious how you came across this issue? Was it reported by someone who had preload turned off?
I think it's generally safe to assume that this feature is enabled, unless you have different information.

Do you have any fallback for IE11 that doesn't support preload?

Blocks: rel=preload
Flags: needinfo?(matt)

Yeah - we had a user report this issue in https://github.com/remix-run/remix/issues/2619. It's very edge case, but is slightly more concerning that their experience is broken instead of just slow. Since it impacts every Remix app - we have added a workaround using timeouts and eventually disables the preloads entirely if enough timeouts are encountered. We wanted to close the loop with you all to see if this was intended behavior and/or if you had other suggestions of how we might handle the scenario.

Flags: needinfo?(matt)

Do you have any fallback for IE11 that doesn't support preload?

Remix doesn't support IE11 in SPA mode since it relies on <script type="module"> and would fall back to SSR only and become a Multi-Page app, so the link preloads would never be leveraged.

This is working as intended I think. network.preload was intended to be used to hide the rel="preload" implementation, so it makes sense that they're treated like any other link.

We should just remove this pref IMO.

Having it disabled is not web compatible, and there's no strong reason
to keep it, IMO.

If we want another pref to determine whether preloads are actually
triggered we can add it in the future.

Assignee: nobody → emilio
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

(In reply to matt from comment #3)

I guess my question is - is there any way to programmatically detect that <link rel="preload" as="style" href="404.css" /> will never emit the load or error event when inserted in the DOM?

Missed this question tho. For what is worth, there is: document.createElement("link").relList.supports("preload")

Severity: -- → S3
Priority: -- → P2
Whiteboard: [necko-triaged]
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/64a1445ac413 Remove network.preload pref. r=necko-reviewers,anti-tracking-reviewers,pbz,kershaw
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41438 for changes under testing/web-platform/tests
Whiteboard: [necko-triaged] → [necko-triaged], [wptsync upstream]
Blocks: old-prefs

document.createElement("link").relList.supports("preload")

Ah thank you! This is exactly what I was looking for - I'm good to go from my end but will leave this open for y'all to close if/after you remove the config.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
Upstream PR merged by moz-wptsync-bot
Flags: qe-verify+

I get same result before and after the fix: no error when loading index.html page on Mac 12.6 using FF builds 115.0 / 118.0a1 (before fix) and 118.0b2(after the fix ).
Reporter, can you please confirm issue is fixed on latest beta (https://archive.mozilla.org/pub/firefox/candidates/118.0b2-candidates/). Thank you so much.

Flags: needinfo?(matt)

Yes, it's fixed. There's no error because the preload succeeds, and the prerequisite for this (toggling network.preload off) is no longer possible.

Flags: needinfo?(matt)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: