1772272 - Support re-snap on CSS transform value changes

Status: NEW

(Core :: Layout: Scrolling and Overflow, defect, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

There's a wpt for re-snapping on transform style value changes, but I am not going to make the test case pass in bug 1530253.

If transform style is animated and if it's running on the compositor, we need to re-snap on the compositor thread. In bug 1530253, we are going to support changes happen on the main-thread. Also I am quite unsure how commonly used transform value changes with scroll-snap.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

CCing evromalarkey since this bug is the reason why the demo in bug 1788029 comment 0 doesn't work.

I've confirmed that following change makes the demo work properly.

diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1712,6 +1712,13 @@ void RestyleManager::ProcessRestyledFram
          nsChangeHint_ChildrenOnlyTransform | nsChangeHint_SchedulePaint)) {
       ApplyRenderingChangeToTree(presContext->PresShell(), frame, hint);
     }
+
+    if (hint & nsChangeHint_RepaintFrame) {
+      // We need to trigger re-snapping to this content if we snapped to the
+      // content on the last scroll operation.
+      ScrollSnapUtils::PostPendingResnapIfNeededFor(frame);
+    }
+
     if ((hint & nsChangeHint_RecomputePosition) && !didReflowThisFrame) {
       // It is possible for this to fall back to a reflow
       if (!RecomputePosition(frame)) {

That being said, I am still wondering this change is half-baked, am wondering whether we end up supporting transform changes in the compositor as well.

evromalarkey, I am sorry for the breakage in your library. Now I am trying to figure out workarounds for your library but I realized that it looks like the slide-in menu works properly without scroll snapping. So I wonder what the purpose of the scroll snap properties in the library is originally. Can you please elaborate it? Thanks!

Comment 2

[evromalarkey]

scroll-snaping is there for seamless touch support on mobile, when you open the menu you can close it by touch via scroll-snap, you can try this on mobile chrome or safari where it works really good (before ff104 it worked good also on ff). The menu is closed only when scroll-snap point is reached.

It's not that bad that it broke the functionality on the library, we can always do workaround there. But the big problem is that the library is used on many websites and these websites menus can't be opened on firefox due to this bug :/

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Note that the diff in comment 1 is wrong. I thought I used hint & nsChangeHint_UpdateTransformLayer. :/ But even with nsChangeHint_UpdateTransformLayer, the test case in bug 1788029 comment 0 doesn't work since it also involves CSS transition: transform which is running on the compositor.

Comment 4

[evromalarkey]

any update on this bug? many websites still have broken menu due to this bug since ff 104 and we have to implement workarounds..

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Hiro, fyi. I agree it's a bit unfortunate that this is a regression / used to work before the scroll-snap changes in 104.

Any other website other than the one from bug 1788029 tho?

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Yeah, this is still (was) on my TODO list, but I couldn't have time to figure out ways to fix the bug 1788029 comment 0 case.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

So from my memory, what the hard part in the particular case is that it has CSS transition: transform, then at the time the transition property is changed, then on the main-thread Gecko computes the transform value is, you know, before-change-style thus we consider the transform isn't changed, then the real transform values are going to be changed on the compositor thread. I guess Chrome uses after-change-style to tell whether re-snap is necessary or not.

Comment 8

[evromalarkey]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

Hiro, fyi. I agree it's a bit unfortunate that this is a regression / used to work before the scroll-snap changes in 104.

Any other website other than the one from bug 1788029 tho?

A lot of our websites at Newlogic Digital - https://www.newlogic.cz/nase-prace, we already hotfixed some of them changing the scroll-snap from mandatory to proximity.

eg. not hotfixed
https://www.siemenspress.cz/
https://www.siemensvyvojar.cz/
https://www.hotelcube.cz/
https://www.nemck.cz/
https://www.mateco.cz/

and a lot more

This one is new and hotfixed https://www.servind.cz/ and also proximity behaves differently on firefox than safari, chrome. If you try to close the drawer with touch, it stays open if you stop in the middle on safari and chrome. This behaviour happens only with proximity, not mandatory on chrome, safari. But firefox behaves the same with proximity / mandatory - doesn't stop in the middle. And same here https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-snap-type

This seems like another bug with scroll-snap, I'm sure that the chrome, safari implementation is correct, is this tracked? :/ happens on versions 105,106,107

Since version 104 scroll-snap in firefox is kind of buggy. In our UI library https://ui.newlogic.cz/ we use scroll-snap for sliders and drawers and this caused us a bunch of issues for firefox users since this update :/

Comment 9

[stuart.tett]

I'm having an issue with resizing the window in Firefox with scroll-snap as seen here: https://codepen.io/tettoffensive/pen/WNgjxaY?editors=1100

When you click on one of the tabs and make the window bigger, then smaller, it doesn't maintain position. It works in Chrome/Safari.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: A test case that a red box position is changed by a CSS transform transition

Surprisingly Chrome handles this case properly, but Safari doesn't. So I've decided to match our behavior to Safari's behavior in bug 1841306 for now, and keep opening this bug for this kind of specific cases.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

(In reply to stuart.tett from comment #9)

I'm having an issue with resizing the window in Firefox with scroll-snap as seen here: https://codepen.io/tettoffensive/pen/WNgjxaY?editors=1100

When you click on one of the tabs and make the window bigger, then smaller, it doesn't maintain position. It works in Chrome/Safari.

Looks like it's a different issue? I haven't debugged into what's going on there. An apparent difference between us and others is we re-snap with smooth scrolling, but others don't. Maybe it's the cause? If you drop scroll-behavior: smooth in the example, it looks it's working properly on Firefox as well.