1165586 - Panning speed increases while urlbar appears/disappears

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Mike Hommey [:glandium]]

STR:
- Open a page that has enough text to pan vertically
- Pan vertically, and note that the text below your finger stays stuck to your finger
- Pan more, such that the url bar disappears, and note that the text below your finger is now moving faster than your finger
- Pan more, after the url bar disappears, and the text below your finger is stuck to your finger again, except it's not the same text as originally, obviously.

Same happens in the other direction.

Comment 1

[Aaron Train [:aaronmt]]

I can't reproduce this on master, maybe fixed via https://github.com/mozilla/firefox-ios/commit/d2c4872294bd9a061cf9b238a92dc8c448379119? Do you still see this?

Comment 2

[Mike Hommey [:glandium]]

Is it in build 17? (I haven't upgraded yet)

Comment 3

[Mike Hommey [:glandium]]

I can reproduce on build 17.

Comment 4

[Mike Hommey [:glandium]]

I can reproduce on build 19.

Comment 5

[Karim Benhmida [:karim]]

What happens is the panning is x2 faster, because it's the sum of the urlbar translation and the webview scroll.
To fix this, we could disable scrolling the webview while the urlbar appears/disappears.

Comment 6

[Stephan Leroux [:sleroux]]

Attachment: PR https://github.com/mozilla/firefox-ios/pull/678

When the user used to scroll, the header would move along with the page content which caused a weird parallax effect and the scroll speed to increase because the web view is being enlarged while also scrolling. For better control over the scrolling, I add our own pan gesture handler on top of the web view's and have them run simultaneously so I can force the scrollView's contentOffset to adjust for the delta change in the frame height. I also moved all of the scrolling behavior code into it's own controller object to clean up the BVC.

Comment 7

[Stephan Leroux [:sleroux]]

Hold off a bit on reviewing this - caught a couple issues last night and will update this morning.

Comment 8

[Brian Nicholson (:bnicholson)]

I hit a pretty nasty layout bug (bug 1181238) while testing this, but that bug is reproducible on master, so this probably isn't related.

I hit another less severe issue:
1) Go to test.com
2) Double-tap to zoom
3) Swipe left and right to overscroll the sides of the page

Following these steps, the overscroll causes the page to flicker.


Another approach that may be worth considering would be disabling the web scroll view as Karim mentioned in comment 5. In particular, we might be able to use shouldRecognizeSimultaneouslyWithGestureRecognizer/shouldBeRequiredToFailByGestureRecognizer to disable the OS panning while scrolling the toolbar, similar to what we do at [1]. I wonder if that approach might simplify things?

[1] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Browser/ContextMenuHelper.swift#L68

Comment 9

[Stephan Leroux [:sleroux]]

Hm not sure what could be causing that over scroll issue. I'm having a hard time reproducing it. 

As for the scrolling, I tried to use the delegate method for failing shouldBeRequiredToFailByGestureRecognizer and also the gesture's requireGestureRecognizerToFail but can't seem to get it to work. The problem I'm having is if I have the scrollView's panGesture only activate when our gesture fails, there is no way to go back. For example, while we're scrolling the headers, we can prevent the scroll view from scrolling. Then after we transition our gesture to a failed state (by setting enabled to false), we can let the scroll view scroll. If the user tries to scroll back up, causing the headers to animate in, there's no way to re-activate our gesture in a continuous way without lifting our finger off to restart the touch events. We could create a custom gesture recognizer and have more control over the toggling of the gesture states but the only way we can turn off the scrollView's panGestureRecognizer is by using the enabled flag. AFAIK, once we disabled it using enabled = false, there's no way to turn it back on and receive touch events from the same continuous touch.

Let me know if you have any ideas -- I've been racking my brain on this one for a good while now.

Comment 10

[Brian Nicholson (:bnicholson)]

Comment on attachment 8630140 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/678

Hm, I see what you mean. This PR works for me!

I'm able to reproduce the flickering side overscrolling glitch fairly reliably with the given STR, so I wish I could be more helpful. Not sure what's going on at first glance.

Comment 11

[Stephan Leroux [:sleroux]]

I'm going to merge this in since it fixes the panning speed issue but I have a WIP patch that will resolve some of the flickering and jumping issues from this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1177112.

Comment 12

[Stephan Leroux [:sleroux]]

Merged.