When scrolling down a webpage vertically, it's possible to enable the stretch overscroll effect, stretching all of the text of the webpage. This is extremely disorienting as I'm trying to read the text while slowly scrolling. I'm assuming this is happening because my finger is not traveling in a straight line. If we can't solve this for 2.1, then I'd recommend removing this for non-fling actions.

Botond do you have any thoughts on this?

(In reply to Kevin Grandon :kgrandon from comment #0) > When scrolling down a webpage vertically, it's possible to enable the > stretch overscroll effect, stretching all of the text of the webpage. This > is extremely disorienting as I'm trying to read the text while slowly > scrolling. > > I'm assuming this is happening because my finger is not traveling in a > straight line. Do you mean the page is stretched horizontally while you are trying to scroll vertically? Is the page scrollable horizontally?

(In reply to Botond Ballo [:botond] from comment #2) > Do you mean the page is stretched horizontally while you are trying to > scroll vertically? Is the page scrollable horizontally? Yes, the page is scrollable horizontally because it's web content, which is too large for the mobile viewport. As I'm scrolling down the page I want to read the text, but the stretching of the text makes that disorienting.

Could you tell me what page and device you are testing with? I tried scrolling some pages on a Flame, and found that the threshold we have before we consider a finger movement to be a pan is sufficiently large that my finger movements scrolling down do not register as horizontal panning. Also, note that the physics related to overscrolling did not change, only the effect. So whatever finger movement triggers the stretch effect in 2.1, would also have triggered the zoom effect in 2.0. Is it the case that you saw the zoom effect in 2.0 under similar circumstances, but found it less distracting than the stretch effect?

I'm seeing this on a flame, on pretty much any site. I think the current stretch effect is very obvious, and it's probably related to the effect and the way that it makes it harder to read the text. My proposed fix would be that we do not apply the stretch effect for a direction unless the delta is some multiplier higher than the movement in the other direction. E.g., if X and Y both moved by 50px, we would not show the effect. If, X > 2 * Y, then we could show the effect in the X axis. Is something like this possible?

(In reply to Kevin Grandon :kgrandon from comment #5) > My proposed fix would be that we do not apply the stretch effect for a > direction unless the delta is some multiplier higher than the movement in > the other direction. > > E.g., if X and Y both moved by 50px, we would not show the effect. If, X > 2 > * Y, then we could show the effect in the X axis. Is something like this > possible? It is. However, we already have a mechanism very similar to what you describe called axis locking (bug 984794) which is not specific to overscroll, and which is enabled on B2G [1]. It would be nice if this existing mechanism would be sufficient, rather than introducing a new mechanism specific to overscroll. I will play around with this tomorrow and see how easy/difficult it is to break out of axis locking. It's possible that our current settings make it too easy, and we need to tweak them. Needinfoing myself so I don't forget. [1] http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js?rev=372ce1f36116#974

(In reply to Botond Ballo [:botond] from comment #6) > I will play around with this tomorrow and see how easy/difficult it is to > break out of axis locking. It's possible that our current settings make it > too easy, and we need to tweak them. Needinfoing myself so I don't forget. I played around with this a bit and found that axis locking successfully prevented me from doing small amounts of horizontal scrolling in cases where I intended to do vertical scrolling. Kevin said on IRC he would post a video of his scrolling; that should shed some light on whether we are doing something differently, or just have different expectations.

Here is a video of the overscroll effect I am talking about when using the phone. I suppose it's only possible to see this when you've zoomed into read something on the phone. This is generally done when there are columns on the website that you want to hide while reading some article or content.

(In reply to Kevin Grandon :kgrandon from comment #8) > Here is a video of the overscroll effect I am talking about when using the > phone. I suppose it's only possible to see this when you've zoomed into read > something on the phone. > > This is generally done when there are columns on the website that you want > to hide while reading some article or content. Looks to me like you're moving your finger horizontally quite a bit when you see the stretching, thus breaking the axis lock. Perhaps that's fine and it should just be more difficult to break the axis lock. CC'ing Gordon to see what he thinks. (My preference is to make the existing axis lock more difficult to break, than to add a more powerful axis lock that's specific to overscrolling, but the latter is possible too.)

Axis locking is working great for me in practice. Not running into the issue that :kgrandon raises in my use of the device. I agree with Botond that tuning axis lock is the right way to improve this. Botond, could their be negative repercussions to making axis lock more "sticky"? I'm not sure what the heuristics are, but I would expect that once an axis is locked (according to some algorithm that detects when locking is desirable), it stays locked for the duration of swipe.

(In reply to Gordon Brander :gordonb from comment #10) > Botond, could their be negative repercussions to making axis lock more > "sticky"? I'm not sure what the heuristics are, but I would expect that once > an axis is locked (according to some algorithm that detects when locking is > desirable), it stays locked for the duration of swipe. We have two axis locking modes, "standard" and "sticky" [1]. In "standard" mode, once you get into an axis-lock, it remains locked for the duration of the swipe. In "sticky" mode, you can break out if you pan enough in the other direction. Currently we have "sticky" mode enabled in B2G. To break out, the vector of your pan needs to make at least a 22.5 degree angle with the locked axis. (I just landed bug 1063227 to make axis-locking constants, including this one, preffable.) What I had in mind for making axis locking stronger, is to increase the break-out angle to a larger value, such as 45 degrees. (A more extreme change would be to switch to "standard" mode, where you can't break out at all.) In terms of negative repercussions for users, I guess it's possible that a user finds it annoying that they have to pan farther in the other direction (or else lift their finger) to break the axis lock. [1] http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js?rev=d59269e27ce4#440

I tried setting the axis breakout angle to 45 degrees, and the results seem reasonable to me. Gordon, would you like to play around with different values of this pref, or should I go ahead and land the 45 degrees and we can look at general user feedback?

I can no longer stand to browse websites on my phone, so I think this is definitely something we should block on. Botond - Can we move forward with landing the 45 degree patch and collect feedback once it's landed?

Sure thing. Note that the stretch effect landed on 2.1, so you might want to consider blocking on 2.1 rather than just 2.2.

(In reply to Botond Ballo [:botond] from comment #14) > Note that the stretch effect landed on 2.1, so you might want to consider > blocking on 2.1 rather than just 2.2. Yes, this is what I meant to do. Thanks.

Trying this on a device, I'm not sure how much this really helps. I think the problem is that I might be starting a horizontal pan *before* axis locking comes into play. I think that potentially having the 45 degree breakout fix might also be nice, if we can solve the initial locking problem.

Comment on attachment 8490157 [details] [diff] [review] Increase axis lock breakout angle to 45 degrees on B2G Clearing review for now; as per IRC discussion yesterday Kevin was going to play around with some of these prefs to see if he could get a better result. Not sure there's a point in landing this change if it doesn't actually fix the problem.

I agree that this is a blocker. The user cannot read.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17) > Comment on attachment 8490157 [details] [diff] [review] > Increase axis lock breakout angle to 45 degrees on B2G > > Clearing review for now; as per IRC discussion yesterday Kevin was going to > play around with some of these prefs to see if he could get a better result. > Not sure there's a point in landing this change if it doesn't actually fix > the problem. I unfortunately was not too successful with these. I think by changing the axis lock to 45 degrees or so things felt better, but then it was quite hard to pan diagonally.

FWIW - This issue also appears on iOS, but it's much less disorienting because the text just moves, instead of stretches.

Given our lack of success in mitigating this problem by tuning axis locking in general, I'm open to considering a solution that's specific to the overscroll effect. Kevin had a suggestion in comment 5, but I'd like to hear from UX before implementing something. Leaving needinfo on Gordon for that.

(In reply to Botond Ballo [:botond] from comment #21) > Given our lack of success in mitigating this problem by tuning axis locking > in general, I'm open to considering a solution that's specific to the > overscroll effect. Make it so http://bit.ly/1wqCwTH ! (In reply to Kevin Grandon :kgrandon from comment #5) > My proposed fix would be that we do not apply the stretch effect for a > direction unless the delta is some multiplier higher than the movement in > the other direction. > > E.g., if X and Y both moved by 50px, we would not show the effect. If, X > 2 > * Y, then we could show the effect in the X axis. Is something like this > possible? A stray thought on this proposed solution: my geometry skillz are... lame, but this sounds like you're suggesting we base the threshold on the angle of the hypotenuse of the scroll..? Sounds very similar to the way I imagine axis locking works. I'm sure Botond has more insight into this than I do, so I trust he'll poke holes in my theory :)

Err, in case Picard didn't make it clear enough: let's get this fixed using whatever approach GFX feels is necessary.

(In reply to Gordon Brander :gordonb from comment #22) > (In reply to Botond Ballo [:botond] from comment #21) > > Given our lack of success in mitigating this problem by tuning axis locking > > in general, I'm open to considering a solution that's specific to the > > overscroll effect. > > Make it so http://bit.ly/1wqCwTH ! > > (In reply to Kevin Grandon :kgrandon from comment #5) > > My proposed fix would be that we do not apply the stretch effect for a > > direction unless the delta is some multiplier higher than the movement in > > the other direction. > > > > E.g., if X and Y both moved by 50px, we would not show the effect. If, X > 2 > > * Y, then we could show the effect in the X axis. Is something like this > > possible? > > A stray thought on this proposed solution: my geometry skillz are... lame, > but this sounds like you're suggesting we base the threshold on the angle of > the hypotenuse of the scroll..? Sounds very similar to the way I imagine > axis locking works. > > I'm sure Botond has more insight into this than I do, so I trust he'll poke > holes in my theory :) Yeah, this suggestion is basically to introduce a separate axis lock mechanism for overscrolling only (so regular scrolling is not affected) where 1) the axis lock is entered automatically at the start of a pan and 2) it's considerably harder to break out. It has an interesting implication for overscrolling near a corner: since X > 2 * Y and Y > 2 * X cannot be true simultaneously, you can never overscroll in both directions at the corner. I'm not sure how desirable that is (this is part of why I wanted UX feedback), but I can try it and see how it feels.

Triage reviewed, blocking+ due to UX & engineering agreeing that this falls into the "broken new feature" category.

This patch makes entering overscroll conditional on the ratio of the pan distance along one axis to the pan distance along another axis being greater than a configurable factor. For example, you can only enter overscroll along the x-axis if X > N * Y, where (X, Y) is the pan distance, and N is a configurable factor. The patch sets N to 1.0 (I personally found this sufficient), but this can be tweaked via the "apz.overscroll.min_pan_distance_ratio" pref. For example, to set it to 2.0, run ./edit-prefs.sh and add the line user_pref("apz.overscroll.min_pan_distance_ratio", "2.0"); Kevin, assuming it's reasonably convenient for you to build a patched Gecko, could you give this a try and let me know if it resolves your problem, perhaps with a tweaked value of the pref?

Comment on attachment 8493829 [details] [diff] [review] Make it harder to trigger the overscroll effect unintentionally Review of attachment 8493829 [details] [diff] [review]: ----------------------------------------------------------------- I've applied the patch and I'm noticing that it's much more difficult to hit the problem I was seeing before, so F+. I still don't think we should ever stretch text though, and possibly consider using some other overscroll effect, but that should be another bug/complaint I guess. Thank you for putting this patch together.

(In reply to Kevin Grandon :kgrandon from comment #27) > I still don't think we should ever > stretch text though, and possibly consider using some other overscroll > effect, but that should be another bug/complaint I guess. Yeah. I would suggest talking to Gordon about this.

This is a small mistake I noticed while debugging this. Tagging along the fix.

Comment on attachment 8493829 [details] [diff] [review] Make it harder to trigger the overscroll effect unintentionally Review of attachment 8493829 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/AsyncPanZoomController.cpp @@ +2108,1 @@ > mY.PanDistance(touchPoint.y)); fix indent

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b87674ddc9c https://hg.mozilla.org/integration/mozilla-inbound/rev/0ecad18902f3

https://hg.mozilla.org/mozilla-central/rev/1b87674ddc9c https://hg.mozilla.org/mozilla-central/rev/0ecad18902f3 https://hg.mozilla.org/mozilla-central/rev/69bcd35607b0 https://hg.mozilla.org/mozilla-central/rev/28f303a138f1

Please request aurora approval on these patches when you get a chance.

Comment on attachment 8493829 [details] [diff] [review] Make it harder to trigger the overscroll effect unintentionally Approval Request Comment [Feature/regressing bug #]: Bug 1057578 (introduction of overscroll stretch effect). [User impact if declined]: On webpages that are scrollable both horizontally and vertically, panning intended to scroll the page vertically can unintentionally overscroll the page horizontally too easily, which is disorienting when trying to read text on the page. [Describe test coverage new/current, TBPL]: Tested locally by Kevin and me, and is now on m-c. [Risks and why]: Relatively low. The change only affects overscrolling, not regular scrolling. An incorrect implementation of this change can lead to getting stuck in an overscrolled state, which renders the page nonresponsive until the display in turned off and on, or the home button is pressed. (I ran into this during my initial attempts to implement the change.) However, such a mistake would very likely be readily apparent, and the patch also adds a debug assertion to help draw attention to such situations. [String/UUID change made/needed]: None.

Comment on attachment 8493829 [details] [diff] [review] Make it harder to trigger the overscroll effect unintentionally Approving and Ni :njpark to make sure this is on his radar to see if there are any fallouts..

Needs rebasing for Aurora uplift.

Actually in master, I can't trigger the horizontal overscroll at all. (just the vertical overscroll) is it disabled or just made it more difficult to trigger?

(In reply to No-Jun Park [:njpark] from comment #37) > Actually in master, I can't trigger the horizontal overscroll at all. (just > the vertical overscroll) is it disabled or just made it more difficult to > trigger? Discussed this with No-Jun in person - the page he was testing was not horizonally scrollable at all. You can only overscroll an element along an axis where the element is scrollable to begin with. We verified that with an element that's scrollable horizontally (e.g. any webpage zoomed in), horizontal overscrolling can be activated if the pan angle is sufficiently close to horizontal.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #36) > Needs rebasing for Aurora uplift. The patch has a code dependency on bug 1066259. It's possible to revise the patch to not depend on this bug, but I think it would be simpler to just uplift the dependency. (There are also dependencies on bug 1063224 and bug 1063227, but those are easy to rebase around. Branch patch coming up.)

Here is the fix, rebased to apply cleanly to aurora on top of bug 1066259's aurora patch. Carrying r+ from kats.

Fixed a couple of compiler errors introduced during rebasing. Carrying r+.

Checked for a couple days since the fix was pushed to master, and there seems to be no fallouts yet.

Is this waiting on anything else before it can be uplifted?

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #43) > Is this waiting on anything else before it can be uplifted? Yes, as described in comments 39-40, this depends on bug 1066259, which is waiting for aurora approval.

Bug 1066259 received aurora approval; the aurora patches there and here (in that order) should be good to uplift.

We can reproduce on Flame v2.1 Steps: 1. Launch Browser. 2. Go to www.yahoo.com and select a new to read. 3. Room in the news. 4. Scrolling down the webpage vertically See attachment(1141.mp4 and logcat_1141.txt) Reproducing rate: 5/5 Flame 2.1 versions: Gaia-Rev 1b231b87aad384842dfc79614b2a9ca68a4b4ff3 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152 Build-ID 20141118001204 Version 34.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141118.035447 FW-Date Tue Nov 18 03:54:58 EST 2014 Bootloader L1TC00011880

Hi Botond, This issue seems still exist with latest version of 2.1. Could you please check again with video: https://bugzilla.mozilla.org/attachment.cgi?id=8525085 Thanks!

Given the angle that the finger is moving on the video I would say that the oversctoll effect is expected in that case. We haven't heard any further complaints on this topic from users so i suspect in practice the behavior is acceptable.

Josh, if you think the behavior is not correct, you should open another bug, and needinfo swilkes@mozilla.com, and she can pass it to the right usability people. We have it to "spec", so if there needs to be a spec change, UX needs to do it.

My fix exposes the threshold angle that's used to determine whether or not you can overscroll as a pref. Currently that pref is 45 degrees, meaning that e.g. if your pan's angle is within 45 degrees of the horizontal axis, you can overscroll horizontally. Changing this threshold to a different value is trivial, but as Milan says the request should go through UX.