core(font-display): limit false positives #9148
Conversation
|
@patrickhulce just for future documentation: this only affects pages that use |
|
@jessepinho yes, that and any other situation like it in which Chrome is unable to provide us with the content of a stylesheet. |
There was a problem hiding this comment.
this is a really nice structural change to the audit; makes perfect sense to do it this way now that I've seen it done this way :)
I feel like you're going to vote 👎 on this idea, but how do you feel about an audit warning on stylesheet content we can't see? :):)
😆 You were very perceptive, I'm not a huge fan since I think it's mostly noise to the user. But how can I say no to a double smiley face!? I suppose I'd also personally find it helpful that I'd have to manually check it too. |
| const fontDisplayMatch = declaration.match(/font-display\s*:\s*(\w+)\s*(;|\})/); | ||
| const rawFontDisplay = (fontDisplayMatch && fontDisplayMatch[1]) || ''; | ||
| const hasPassingFontDisplay = PASSING_FONT_DISPLAY_REGEX.test(rawFontDisplay); | ||
| const targetURLSet = hasPassingFontDisplay ? passingURLs : failingURLs; |
There was a problem hiding this comment.
this is also why I didn't do it the first time around lol 😆
Co-Authored-By: Brendan Kenny <[email protected]>
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
👍 to @brendankenny 's comment re: the audit warning. Would be nice to know that Lighthouse is unable to see my stylesheets! |
|
Yep, @jessepinho this has been implemented |
|
@patrickhulce awesome, thanks for merging! when would I expect to see this in Chrome/PageSpeed Insights? |
|
@jessepinho expect it to be in the next released version of LH (so ~few weeks) at which point it will be reflected in PSI. Canary Chrome should be soon after that but don't hold your breath for Chrome stable. It will go out with Chrome 77 at the earliest which becomes stable Sept 10. |
|
@patrickhulce Thank you! |
Summary
In font-display, sometimes we just don't get the content of stylesheets for whatever reason. When this happens we falsely flag fonts as not having
font-displayvalues when they do, we just can't see them.This PR inverts the logic in the audit to limit false positives. Instead of finding a set of passing URLs and failing everything else, we find the set of failing URLs directly.
Related Issues/PRs
addresses user-facing concerns of #8493