Closed Bug 1709232 Opened 2 months ago Closed 1 month ago

Firefox falsely claims it supports the first-input and event performance entry types

Categories

(Core :: Performance, defect)

Firefox 88
defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: philip, Assigned: sefeng)

Details

Attachments

(1 file)

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

Steps to reproduce:

Run the following code in the developer console:

console.log(PerformanceObserver.supportedEntryTypes.includes('first-input'));
console.log('PerformanceEventTiming' in self);

Actual results:

The following is logged:

// true
// false

Expected results:

Both log statements should be the same (either true or false).

The different means that Firefox passes feature detects for the First Input Delay as well as Event Timing usage, but it does not actually support it.

Note: this issue just seems to affect Firefox 88, when testing Firefox 89 it works as expected.

Note, this issue is causing the FID polyfill in the web-vitals JS library to not work in Firefox 88 because the feature detect passes but native support doesn't actually work: https://github.com/GoogleChrome/web-vitals/issues/142

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

Component: Untriaged → DOM: Events
Product: Firefox → Core
Assignee: nobody → sefeng
Component: DOM: Events → Performance

Yeah, this is indeed a bug in Firefox 88, I didn't implement a pref guard for PerformanceObserver.supportedEntryTypes, so it returned true even if the pref is disabled.

event-timing is going to be fully supported in Firefox 89, so that explains why Firefox 89 works as expected.

PerformanceObserver.supportedEntryTypes doesn't check whether
PerformanceEventTiming is enabled or not, so it'll falsely claims
PerformanceEventTiming is supported even if the pref is disabled.

This patch fixes that by adding a guard to it.

Thanks Sean! Quick question: will this patch be merged into Firefox 88? And I suppose a larger question if the answer is no, is there a chance that an ESR version of Firefox will contain this bug?

Yeah, we probably are not going to uplift this patch into Firefox 88. I could always try, however I don't see this is urgent enough for a release uplift. Let me know if you have different opinions.

This patch should be able to make into Firefox 90 (I said should...however it should be like 100%), and the next ESR is 91, so ESR version of firefox will not contain this bug. (Current ESR is Firefox 78, which has no event timing implementation at all).

I don't see this is urgent enough for a release uplift. Let me know if you have different opinions.

I agree it's not urgent. I've already fixed the feature detect in the web-vitals JS library, so it's not a huge issue there. My main concern was that Firefox 88 didn't end up be the ESR version. I saw current ESR is 78 and previous was 68, so I got a bit concerned :)

If the plan is for ESR to be 91, then I'm 100% fine with not patching 88. Thanks again!

Pushed by sefeng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/809f11778c39
Make PerformanceObserver.supportedEntryTypes respects dom.enable_event_timing r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28845 for changes under testing/web-platform/tests
Status: UNCONFIRMED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.