search

GoogleChrome / web-vitals

Essential metrics for a healthy site.
https://web.dev/vitals
Apache License 2.0
7.59k stars 415 forks source link

onLCP may report even page was hidden previously #325

Closed zuoaoyuan closed 1 year ago

zuoaoyuan commented 1 year ago

Some metrics like LCP & FID only report if the page wasn't hidden prior to the metrics. FID: https://github.com/GoogleChrome/web-vitals/blob/main/src/onFID.ts#L59 LCP: https://github.com/GoogleChrome/web-vitals/blob/main/src/onLCP.ts#L63-L66

However, in case ActivationStart has a non-zero value like pre-render, LCP compare with the page hidden time after subtracting the activation time . Shouldn't we compare before substract?

zuoaoyuan commented 1 year ago

@tunetheweb Hey Barry, can you help confirm since you're the prerender expert:)

tunetheweb commented 1 year ago

Yes I think you are correct that this should only be reported if the lastEntry.startTime was before the firstHiddenTime.

However, prerendered pages cannot currently be activated as background tabs (the prerender is currently discarded and the page is loaded as a fresh navigation if someone does right-click->open in a new tab). So this only affects if the page was prerendered AND the LCP was NOT finished by the time the prerender was activated (so the prerender wasn't completed before activation) AND the page was then hidden before the LCP was showing AND the page was then reactivated AND the LCP was shown AND this happened in a time that allowed it to report. I'd imagine that's reasonable rare.

Take these examples:

Activation = 1 second Hidden = 1.2 second Unhidden 2.0 seconds LCP = 2.5 second

In that case the check would be that 1.5 seconds < 1.2 seconds, which fails, so the event would not be reported.

However in this case, the current logic is wrong:

Activation = 1 second Hidden = 1.2 second Unhidden 1.4 seconds LCP = 1.9 second

In that case it would be 0.9 seconds < 1.2 seconds, which succeeds, to the event would be reported (even though it shouldn't). But it does depend on hiding and then unhiding a page really quickly.

But it's still incorrect and an easy fix so will submit a PR now.

tunetheweb commented 1 year ago

Happy with the change in #326 ?

zuoaoyuan commented 1 year ago

That would be a super rare case I agree. And the fix looks great, thanks!