Closed Bug 1050789 Opened 8 years ago Closed 8 years ago

setting overflow-y:hidden during overscrolling animation breaks touch events

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1041471

People

(Reporter: mmedeiros, Unassigned)

References

Details

Attachments

(5 files)

I need to lock the vertical scroll while the user is doing an "horizontal drag" so I'm setting `overflow-y:hidden` during the first `touchmove` that the X distance is at least 10px bigger than the Y distance (I consider it as an horizontal drag). And I reset the 'overflow-y` to `scroll' on `touchend|touchcancel`. 

This works great if overscrolling is disabled or if the overscrolling animation is not executing (scroll < scrollTopMax && scroll > 0).

If I toggle the overflow-y while the overscrolling is happening (scroll > scrollTopMax || scroll < 0) it breaks the touch events; touchstart, touchmove, touchend are not fired and I can't scroll the element anymore.

I have a DOM structure like `#touch-target > #header + #main`, where both `#header` and `#main` have `overflow-y:scroll`, if the overscrolling is happening on `#main` when I reproduce the bug the `#header` still bubbles the touch events, while `#main` "swallows" all the events... If I toggle the display of the `#touch-target` (set to `none` then `block`) the touch events (and APZ) starts working again.

It looks like an edge case but it is not that hard to reproduce the bug by mistake..

I would be happy with a fix to this problem or with a new CSS property that disables the overscrolling for a single element. - overscrolling doesn't "feel right" on our app since we have a fixed header that doesn't scroll together with the content (it creates a weird gap between content and header).

PS: I need to do the "horizontal drag" with JS since I need to snap the scroll to a grid and I really can't let the user overscroll or do multiple swipes in a row - need to add/remove grid columns dynamically, if user scrolls fast he would see a blank screen...
Blocks: 1023662
I have no idea how APZ works, but it feels like it is adding a new element on top of my content (used for the animation) and that this element is not being removed since the animation never finishes (so it blocks the touch interaction of elements underneath it). - it's just a guess, but maybe this helps someone to understand what is going on.
(In reply to Miller Medeiros [:millermedeiros] from comment #0)
> I need to lock the vertical scroll while the user is doing an "horizontal
> drag"

I know this isn't helpful at the moment but once we hook up the touch-action CSS property on B2G (bug 960209) that would be the "correct" way to solve this problem.

> so I'm setting `overflow-y:hidden` during the first `touchmove` that
> the X distance is at least 10px bigger than the Y distance (I consider it as
> an horizontal drag). And I reset the 'overflow-y` to `scroll' on
> `touchend|touchcancel`. 
> 
> This works great if overscrolling is disabled or if the overscrolling
> animation is not executing (scroll < scrollTopMax && scroll > 0).
> 
> If I toggle the overflow-y while the overscrolling is happening (scroll >
> scrollTopMax || scroll < 0) it breaks the touch events; touchstart,
> touchmove, touchend are not fired and I can't scroll the element anymore.

In general we don't deliver events to content if any of the elements that the touch events are passing through is in overscroll. It sounds like what is happening here is that we're getting "stuck" in the overscroll state when you change the overflow-y, and so we stop firing the touch events. This is a bug; when you change the overflow-y we should be clearing the overscroll immediately and delivering touch events normally, I think. Botond would be able to provide more info here, although we might need a test case to reproduce this to be sure.

> I have a DOM structure like `#touch-target > #header + #main`, where both
> `#header` and `#main` have `overflow-y:scroll`, if the overscrolling is
> happening on `#main` when I reproduce the bug the `#header` still bubbles
> the touch events, while `#main` "swallows" all the events... If I toggle the
> display of the `#touch-target` (set to `none` then `block`) the touch events
> (and APZ) starts working again.

This is consistent with what I think is happening as described above. Changing the display of the touch-target would rebuild the layer tree and reset a bunch of state.

> I would be happy with a fix to this problem or with a new CSS property that
> disables the overscrolling for a single element. - overscrolling doesn't
> "feel right" on our app since we have a fixed header that doesn't scroll
> together with the content (it creates a weird gap between content and
> header).

This is something that has come up a couple of times now, and it might be worth adding. I'll file a bug to discuss it more detail, as it's separate from this bug.

> PS: I need to do the "horizontal drag" with JS since I need to snap the
> scroll to a grid and I really can't let the user overscroll or do multiple
> swipes in a row - need to add/remove grid columns dynamically, if user
> scrolls fast he would see a blank screen...

Just to be clear, you're not actually setting the scrollLeft/scrollTop values from JS, right? You're just tracking the normal scroll movement using touch events and then doing axis-locking by modifying the overflow-y property?
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> (In reply to Miller Medeiros [:millermedeiros] from comment #0)
> > PS: I need to do the "horizontal drag" with JS since I need to snap the
> > scroll to a grid and I really can't let the user overscroll or do multiple
> > swipes in a row - need to add/remove grid columns dynamically, if user
> > scrolls fast he would see a blank screen...
> 
> Just to be clear, you're not actually setting the scrollLeft/scrollTop
> values from JS, right? You're just tracking the normal scroll movement using
> touch events and then doing axis-locking by modifying the overflow-y
> property?

Yes, I'm not setting the scrollLeft/scrollTop. I let the overflow-y handle the vertical scroll, and the horizontal scroll I do with CSS transforms (I scroll my element between 0px and -580px and reset the translateX to -290px every time the user releases). - right now I do all the CSS transforms updates on requestAnimationFrame since I need to keep a "smooth" transition (I do some custom easing based on distance/time...) but I might change it to CSS transitions later.
PS: the `translateX` is on the content that is inside the #main and #header (both have overflow-y:scroll; overflow-x:hidden - should scroll both elements in sync, that's why I add the listener to the parent element.
Attached file simplified test case
I created a simplified test case of what I have right now (removed all the animation/snapping logic and simplified the DOM structure as well). This should help you debug.

if you remove a few paragraphs from the #main-content it will be easier to reproduce the bug since overscrolling happens more often (sometimes I was getting it even when doing just the horizontal scroll). But it should be easier to reproduce it anyway.. notice that touch events on the #header always bubble (since I don't toggle the overflow-y).
(In reply to Miller Medeiros [:millermedeiros] from comment #5)
> Created attachment 8470138 [details]
> simplified test case

When I scroll the main div horizontally, the scale changes to be very small, as if a transform:scale(0.25) or something like that were applied to the content. Strange, since the only transform I see the code setting is a translateX.
(In reply to Botond Ballo [:botond] from comment #6)
> When I scroll the main div horizontally, the scale changes to be very small,
> as if a transform:scale(0.25) or something like that were applied to the
> content. Strange, since the only transform I see the code setting is a
> translateX.

Looking more closely, the text is being reflowed, so it's not an issue with transforms.
Here are before / after display list dumps that show the text being reflowed.
And frame dumps.
Attachment #8470363 - Flags: feedback?(tnikkel)
Comment on attachment 8470363 [details]
Frame dump after (text is tiny)

The text frames are smaller in the after dump.

I don't see anything in the testcase (http://people.mozilla.org/~bballo/apz-overscroll/index.html) that would be mobile specific, other than the touch events, which we should be able to change to something suitable to trigger the same bug on desktop. Reducing the testcase further might help point a finger at something.
Attachment #8470363 - Flags: feedback?(tnikkel)
weird.. I tested it on Flame (master build from a few days ago), installed the app through the Web IDE. Was not able to reproduce the "tiny text"... anyway, the bug happens with or without text, I added the text just because it is easier to see the scroll and because I had a code snippet to add the lorem ipsum.
(In reply to Botond Ballo [:botond] from comment #6)
> When I scroll the main div horizontally, the scale changes to be very small,
> as if a transform:scale(0.25) or something like that were applied to the
> content. Strange, since the only transform I see the code setting is a
> translateX.

I see this as well, but it's not a transform:scale(0.25). My guess is that setting 'hidden' as overflow-y property changes the font inflation heuristics outcome, and we stop doing font inflation on this text. So then it renders at its un-inflated size which is tiny.

To remove this effect it might be a good idea to add a <meta name="viewport" content="width=device-width"> to the standalone test page (http://people.mozilla.org/~bballo/apz-overscroll/index.html) which will disable font inflation entirely. I haven't checked but the packaged web app has this viewport set by default (even if it's not explicitly stated) and so font inflation will be disabled there.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> (In reply to Botond Ballo [:botond] from comment #6)
> > When I scroll the main div horizontally, the scale changes to be very small,
> > as if a transform:scale(0.25) or something like that were applied to the
> > content. Strange, since the only transform I see the code setting is a
> > translateX.
> 
> I see this as well, but it's not a transform:scale(0.25). My guess is that
> setting 'hidden' as overflow-y property changes the font inflation
> heuristics outcome, and we stop doing font inflation on this text. So then
> it renders at its un-inflated size which is tiny.
> 
> To remove this effect it might be a good idea to add a <meta name="viewport"
> content="width=device-width"> to the standalone test page
> (http://people.mozilla.org/~bballo/apz-overscroll/index.html) which will
> disable font inflation entirely. 

Indeed, adding <meta name="viewport" content="width=device-width"> makes the font-size-change issue go away.

> I haven't checked but the packaged web app
> has this viewport set by default (even if it's not explicitly stated) and so
> font inflation will be disabled there.

This explains why I was seeing this issue but Miller wasn't; I was browsing the index.html from his test case in the Browser app, while he probably had it installed as a packaged app.
Depends on: 1052023
While interacting with this page, I ran into an APZ assertion which may possibly be the cause of the problem. I filed bug 1052023 for it.
As dicussed with Miller on IRC, he was testing with a Gecko from 07/23, and cannot reproduce the problem with today's Gecko (nor can I). That makes this a likely duplicate of bug 1041471, which landed that day (and would have only made it into the 07/24 nightly).

That said, I can reproduce a slightly different problem, where if you open the page in the Browser app - where you can zoom it - and zoom it, you can then get stuck in overscroll. I filed bug 1052121 for this.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1041471
You need to log in before you can comment on or make changes to this bug.