Closed Bug 1519371 Opened 3 years ago Closed 3 years ago

Reframe after adding `will-change: transform;` breaks scroll event handler


(Core :: Layout, defect)

Not set



Tracking Status
firefox-esr60 --- unaffected
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed


(Reporter: denschub, Assigned: emilio)




(Keywords: regression)


(1 file)

This was originally filed as a WebCompat bug, but it looks like this is a regression of bug 1479859.

The site in question has a full-viewport overlay that gets scrolled away via JS on the scroll event. When dragging starts, the site adds a CSS class that adds will-change: transform to the element, which sometimes results in us no longer processing the scroll events, and the element not getting scrolled away.

Flags: webcompat?

Emilio, since you helped in the WebCompat bug, is this something you can look at?

Flags: needinfo?(emilio)

Is there any chance you could try to find a bit more reduced test-case for this? I tried to come up with one, but couldn't (latest attempt is

If the answer is not is ok, I'll take a quick look at rr today, but if I don't figure out I might need to move on, since I'm swamped with other stuff :(

Component: CSS Parsing and Computation → Layout
Flags: needinfo?(dschubert)

Is there any chance you could try to find a bit more reduced test-case for this?

I'll give it a try. Let's cancel your ni? for now, I'll ping back if I have a testcase (or when I have given up. :))

Flags: needinfo?(emilio)


So the issue here is that adding will-change: transform to an already-transformed frame will reframe it.

I think there's no underlying correctness issue, but reframing in this case is inefficient and results in this havoc.

We should be able to do better.

Flags: needinfo?(emilio)
This patch should make the detection of whether we should reframe in
UpdateContainingBlock exact.

It should have no behavior change, but sometimes reframing can confuse event
handling code or what not.

We don't have a reduced test-case for the event handling regression this fixes,
but I added a test to ensure we don't uselessly reframe in this case that fails
without this patch and passes with.
Pushed by
Reframe less as a result of UpdateContainingBlock. r=dholbert
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Flags: needinfo?(dschubert)
You need to log in before you can comment on or make changes to this bug.