Closed Bug 721125 Opened 8 years ago Closed 8 years ago

Zooming out and scrolling resulted in viewport going black

Categories

(Firefox for Android :: General, defect, P1)

All
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 12
Tracking Status
firefox11 --- verified
firefox12 --- verified
fennec 11+ ---

People

(Reporter: kats, Assigned: kats)

Details

(Whiteboard: [MTD])

Attachments

(2 files, 2 obsolete files)

Attached file logcat from edmorley (obsolete) —
edmorley reported on IRC:

at the time I was panning the latest memshrink blog post up and down (since it was being a bit janky), pinching to zoom out, to see if I could scroll to the top more quickly whilst in the  zoomed out state, at which point the content area just turned black

partial logcat attached, it shows the viewport has a lot of NaNs but doesn't go back far enough to point out exactly what caused it to get into that state.
tracking-fennec: --- → ?
Attached file Log (obsolete) —
Have reproduced whilst logcat was running, so have the full log now.

Rough STR:
1) Go to http://blog.mozilla.com/nnethercote/2012/01/25/memshrink-progress-week-32/
2) Alternate pinch to zoom out / scroll up and down / pinch to zoom out but leave one finger still on screen, so doesn't zoom back in again, whilst trying to zoom out more & scroll page offscreen
3) Repeat #2 until viewport goes black

At which point reloading the page / opening a new tab doesn't help, all other pages (apart from about:home) display the black viewport until Fennec is force quit.

I suspect this originally occurred in comment 0 by my holding the device along the left edge, such that my hand was accidentally touching the screen and thus causing false pinch actions when I was only trying to scroll (it wasn't as contrived as the STR/log in this comment).

Galaxy S2, Android 2.3.4, Fennec native 12.0a1 2012-01-24.
Attachment #591549 - Attachment is obsolete: true
Whiteboard: [MTD]
Wow, the system ScaleGestureDetector is completely ballistic, it's giving us negative distance values for the pinch-span as seen in this line:

D/GeckoLayerController(31050): scaleWithFocus: v=RectF(93350.98, -382394.47, 93830.98, -381695.47) p=(-17174.494,-488614.3) z=-17.524994 o=0.0,0.0; zf=-17.524994

However bug 706684 (which went in yesterday and isn't in your build, I believe) replaces the system ScaleGestureDetector with our own, and should fix this problem. There have been other bugs (717704, 718970) which also show weird behaviour when touching on or around the edge of the screen, and which have also been resolved with the rewrite.

I'll leave this open for now but please re-test in the most recently nightly as this should be fixed. I might stick in some code to try and guard against this situation as well in case we have to switch back to the system SGD.
Updated to latest m-c nightly (https://hg.mozilla.org/mozilla-central/rev/0d5ad6a6f814) and was able to repro - new log attached.

More precise STR:
1) Navigate to http://blog.mozilla.com/nnethercote/2012/01/25/memshrink-progress-week-32/ (though seems to happen on any page)
2) Press and hold thumb vertically along left edge of screen, roughly 30-40% of the way up from the bottom
3) Whilst still holding thumb, repeatedly (say 8-10 times) tap, slide and release finger from top right of the screen towards ~thumb/the bottom left, until viewport turns black.
Attachment #591576 - Attachment is obsolete: true
tracking-fennec: ? → 11+
Priority: -- → P1
Attached patch PatchSplinter Review
While in pinch, it's possible to pan the page completely out of the viewport and this causes getEdgeResistance() to return negative values. When spanRatio > ~2 the resistance on it can make it negative in onScale() which then does all sorts of damage. This patch should prevent getEdgeResistance() from returning a negative value.

I also posted a build of (7ab255f53568 + this patch) at https://people.mozilla.com/~kgupta/tmp/bug721125.apk - :edmorley if you want to try it out and let me know if you still see the problem that would be great.
Attachment #592276 - Flags: review?(pwalton)
Haven't been able to reproduce with this patch :-)
Comment on attachment 592276 [details] [diff] [review]
Patch

Review of attachment 592276 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit addressed.

::: mobile/android/base/ui/Axis.java
@@ +189,5 @@
> +            // excess can be greater than viewport length, but the resistance
> +            // must never drop below 0.0
> +            return Math.max(0.0f, SNAP_LIMIT - excess / getViewportLength());
> +        } else {
> +            return 1.0f;

nit: unnecessary else after return
Attachment #592276 - Flags: review?(pwalton) → review+
https://hg.mozilla.org/mozilla-central/rev/4b41e38a1f13
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 592276 [details] [diff] [review]
Patch

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: Could end up in a state where no tabs render properly, can only be fixed by restarting fennec
Testing completed (on m-c, etc.): on m-c, test build with patch
Risk to taking this patch (and alternatives if risky): low risk of regressions in panning resistance, mobile only.
String changes made by this patch: none
Attachment #592276 - Flags: approval-mozilla-aurora?
Comment on attachment 592276 [details] [diff] [review]
Patch

Moving approval request from aurora (now 12) to beta (now 11) since this patch is in 12 already.
Attachment #592276 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 592276 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Beta 11.
Attachment #592276 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Following the steps from comment 1 and 3 I didn't saw any black content area.

Verified fixed on build:
Firefox 11 (tinderbox build): 	1328588649/	06-Feb-2012 21:51 		
Firefox 12: Firefox 12.0a2 (2012-02-06)
Firefox 13: Firefox 13.0a1 (2012-02-06)

Device: LG Optimus 2X (Android 2.2.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.