Font style sheets in AMP misclassified as render blocking.

[cramforce]

Lighthouse consider a link tag like <link href="https://fonts.googleapis.com/css?family=Roboto:100,300,400" rel="stylesheet"> in AMP documents as render blocking.

But that is wrong.

Because AMP uses this async loaded script to remove the link tag and asynchronously load the link tag it if it hasn't been loaded within 1 second of the time of first byte.

See https://github.com/ampproject/amphtml/blob/master/src/font-stylesheet-timeout.js#L39

This might be hard to detect, but it might be misleading for a lot of people.

[paulirish]

Thanks for reporting this. Appreciate it.

From our read, it does appear that these stylesheets will be render-blocking for 1 second, but they'll flip after that. Lighthouse can't account for this flip, but I can see how that'd be useful.

For it to be fairly reliable, we'd want to know when the media attribute changes to be something non-standard, which is used for the non-render blocking hack (also used in the loadCSS project). I tried this out, attempting to detect this moment via a Proxy that I installed to the HTMLLinkElement.prototype however it didn't work. Will need some more investigation.

In the ideal, Lighthouse detects that these stylesheets are render-blocking for the first second but then not after that. This indeed means they shouldn't be a major perf issue for most sites.

---

Also notable, is that in Chrome, ANY stylesheet added to the DOM via a script (createElement/appendChild) is not treated as render blocking. This is kind of surprising but you can see it here: http://stevesouders.com/cuzillion/?c0=hc1dfff2_0_f&c1=bi1hfff0_0_f&t=1501704822
Same with Firefox (I think). Meanwhile, Safari does consider these to be render blocking.

[paulirish]

I tried this out, attempting to detect this moment via a Proxy that I installed to the HTMLLinkElement.prototype however it didn't work. W

Fell back to good ole defineProperty and found success.

Object.defineProperty(HTMLLinkElement.prototype, 'media', {
  set: function(val){
    // TODO...
    // if val is a "valid media query list", then the stylesheet is render blocking (in some browsers/cases)
    // if its not valid, then mark it as non-render blocking.
    // spec jumpoff point: https://html.spec.whatwg.org/dev/common-microsyntaxes.html#mq

	this.setAttribute('media', val);
  },
  get: function() {
    this.getAttribute('media');
  }
});

[FinnBorge]

Just wanted to +1 this. Spent a while trying to figure out how it was possible that the amp-font was render blocking. Delighted to see that it is not.

[westonruter]

As @choumx notes in #10046 (comment), the fix in #4555 doesn't seem to be working anymore. I'm getting “Eliminate render-blocking resources” audit failures for a Google Font on this AMP page: https://elsoberano.org/

Lighthouse (v5.2.0) ran with configuration:

[luckyankit]

I am also getting the same error as above for https://www.reviewcenter.in/. It's also on full AMP using newspack theme. Lighthouse also recommends sometimes to preconnect to origins which I've already added to connect to.

[zachnicodemous]

Confirming this issue is flagging on several AMP powered sites I manage too. PageSpeed is flagging any amp-font I call as render blocking.

[zachnicodemous]

Is there any solution for this? Its having a catastrophic impact on at least one of my clients websites.

[patrickhulce]

Our plan here is to use the stack pack detection for AMP pages and manually cap the reported render blocking time at the max of AMP's threshold and the observed request time when throttlingMethod: 'simulate'.

Rationale: We will never be able to know when throttlingMethod === 'simulate' what would have happened in JS if the request took more time in the conditions we're simulating.

If there's other frameworks that have similar behavior we're missing that maintain a stack pack that need this too, we're def open to those too not just AMP :)

We'd also welcome the AMP team to consider if this stylesheet timeout pattern could become a web standard (a la font-display). cc @choumx

[paulirish]

@adamraine is gonna dig in with @patrickhulce's assistance.

[patrickhulce]

We consider this fixed by #11069.

Note that many sites are still going to display "Render-blocking resources" audit results on AMP pages. This audit will still produce results because AMP's method does still use render-blocking requests, it just limits the impact they have to ~2.1s. Since most of the comments in this issue are about results that are less than this threshold, I wouldn't expect many reports to be that different, but at least know our results (as of LH ~v6.2) are now inline with AMP's :)