If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[regression] MediaQueryList: event is dispatched only once

NEW
Assigned to

Status

()

Core
DOM: CSS Object Model
P3
normal
3 months ago
17 hours ago

People

(Reporter: zer0, Assigned: bradwerth)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(firefox57 wontfix, firefox58 affected)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Reporter)

Description

3 months ago
Created attachment 8881951 [details]
MediaQueryList test

Open the attachment, and zoom in / zoom out.

Expected Result:
 - At each zoom, the previous and current devicePixelRatio is logged in the page.

Actual Result:
 - Only the first time the devicePixelRatio is logged (sometimes it was logged twice, but I'm not able to reproduce it all the times).

I'm not sure when this code stop working, at least from 53.

At least Responsive Design Mode is using the MediaQueryList in this way to detected when the device pixel ratio is changing, and we also have tests that do so.
(Assignee)

Updated

3 months ago
Assignee: nobody → bwerth
Component: Layout → DOM: CSS Object Model
This doesn't seem to be a regression... I can reproduce this issue as described with the STR back to at least Firefox 30 (with a modified version to desugar ES6 features).

It isn't clear to me which in version were resolution and devicePixelRatio implemented, but I would guess this doesn't work from the very beginning...
Haven't had a close look, but this *might* be the restyle bug we're hitting in bug 1358688

Updated

14 days ago
Priority: -- → P3
(Assignee)

Comment 3

3 days ago
This problem is happening because the notification loop for the MediaQueryLists can't handle adding another MediaQueryList during iteration. That happens when another matchMedia() call happens in the event listener. I'll change the notfication loop to copy the lists before iterating, and that should take care of it.
(Assignee)

Comment 4

23 hours ago
The issue in comment 3 is part of the problem, but not all of it. Another part of the problem is that we have a float precision comparison problem. In this case, when the MediaQueryList is generated for the 120% size, near 2.4 dppx, we end up doing a comparison that is close but not exact. The nearly 2.4 number is multiplied by 96 pixels per inch, and so we end comparing....

230.399994 from the MQL

vs

230.400009 from the PresContext.

Those aren't the same numbers, but the current code requires them to be the same for sensible things to happen. As it is, this comparison first happens whenthe MQL is given its first listener, and the code concludes (incorrectly) that the MQL does not match the current DPR, though it was created from the window.devicePixelRatio and should match. Later on, when the DPR does change, the MQL sees that it already didn't match, and doesn't match the current, so no need to fire a DPR change event.

So the second part of the solution will be either enforcing some kind of float precision in the calculation, or relaxing the epsilon of the comparison. My preference is to relax the comparison. Within 1e-5 would do it. I'll submit a patch that does that.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

19 hours ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2738895e6980eda0fa8fb0a7154acc3c05ab478a
(Assignee)

Updated

19 hours ago
Attachment #8910986 - Flags: review?(cam)
Attachment #8910967 - Flags: review?(cam)
Attachment #8910966 - Flags: review?(cam)

Updated

17 hours ago
status-firefox57: --- → wontfix
status-firefox58: --- → affected
You need to log in before you can comment on or make changes to this bug.