Closed Bug 1259735 Opened 5 years ago Closed 5 years ago

Android: Scrolling Gets Stuck Completely on Certain Sites

Categories

(Firefox for Android :: Toolbar, defect)

Product:
Firefox for Android
Component:
Toolbar
Version:
45 Branch
Platform:
Unspecified
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 48
Tracking Flags:
Tracking Status
firefox45 --- wontfix
firefox46 --- verified
firefox47 --- verified
firefox48 --- verified

People

(Reporter: dontarius, Assigned: kats)

References

Details

(Whiteboard: [JPZC scrolling])

Attachments

(2 files)

Firefox_Android_Bug_Scrolling.mp4
2.63 MB, video/mp4
Details
MozReview Request: Bug 1259735 - Guard against a rounding error that could lead to Java having a stale page size, rendering the page unscrollable. r?snorp
58 bytes, text/x-review-board-request
snorp
: review+
ritu
: approval-mozilla-aurora+
lizzard
: approval-mozilla-beta+
Details 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 2016030500
Firefox for Android

Steps to reproduce:

The following procedure allows me to reproduce this bug 100% of the time.  If I alter the procedure (e.g. I do not use a new guest session or use other sites), then the bug may or may not occur.

On FF Android 43.0 - 45.0:
1) Start a new Guest Session
2) Go to html5test.com (mobile version)
3) Click on "other browsers" link at the top
4) Try to scroll -- it does not work at all (page is completely stuck)

Additional Notes:  I'm on a Nexus 4 with Android 4.4.4.  Scrolling used to work perfectly, until Firefox 43, I believe.  Since version 43 (not completely certain that is the version where it first appeared) I've had this issue.  I've only noticed this problem on mobile sites, but it has been pretty pervasive (i.e. I have the problem on many, many sites).  However, it seems more prevalent on a first visit (hence the Guest Session in the steps above).

I am unable to use the same steps to reproduce the problem on another person's Nexus 6 (though they have also pointed out issues with scrolling in recent FIrefox versions).



Actual results:

The ability to scroll is completely gone, and no amount of waiting will bring it back.  The only way I have found to be able to scroll again is to go to a different tab in FF and then return to the tab where scrolling malfunctioned.

A video demonstration of the bug is attached.


Expected results:

Scrolling
OS: Unspecified → Android
Component: Untriaged → Graphics, Panning and Zooming
Product: Firefox → Firefox for Android
I can not reproduce on a Nexus 5x. Will try to get my hands on a Nexus 4. Would you try Firefox nightly? https://nightly.mozilla.org
Flags: needinfo?(kbrosnan)
Flags: needinfo?(dontarius)
I cannot reproduce this bug using the latest nightly.  I actually tried the nightly when this issue first came up around the release of FF 43, and even then I could not reproduce in the nightly.  Hence, I never reported the bug and expected it to be fixed in the next release or two.  However, I do still see the bug in FF46.0b4, so I'm not sure how long it takes for things to move from nightly to beta / release.
Flags: needinfo?(dontarius)
It might be that it is fixed in Nightly because of APZ, but we haven't allowed APZ to ride the trains to aurora/beta yet.

(In reply to dontarius from comment #0)
> On FF Android 43.0 - 45.0:
> 1) Start a new Guest Session
> 2) Go to html5test.com (mobile version)
> 3) Click on "other browsers" link at the top
> 4) Try to scroll -- it does not work at all (page is completely stuck)

I tried these STR on a Nexus 4 running Android 4.2.2 and was not able to repro on Firefox 44 or 45.0.1.
I can confirm that I still see the bug in aurora 47.0a2.  Also, the bug only occurs in portrait mode.  If scrolling is stuck and I rotate to landscape, I am able to scroll.  Rotating back to portrait allows me to then keep scrolling.
I did run into this problem yesterday evening in Aurora as well, randomly. Based on the symptoms it looked like the Java side of the code didn't get an updated page size after the page was done loading, so it thought the page was just one screen long. Zooming in and out showed that the scrollbar reflected this, and that also explains why rotating the device fixes the problem.
Status: UNCONFIRMED → NEW
Ever confirmed: true
status-firefox45: --- → affected
status-firefox46: --- → affected
status-firefox47: --- → affected
Whiteboard: [JPZC scrolling]
I think you might be on to something there in terms of the scroll bars being a symptom indicating what the underlying problem might be.  I tried zooming as well and the scroll bars initially suggest the page is only one screen long.  Only when I pan out of the one screen area do the bars reset to indicate the page is larger.

Other interesting facts:
1) If I go to html5test.com/results/desktop.html directly (ie not via link on html5test.com) then the bug does not occur.
2) If I load the page in landscape mode from the beginning, then the bug does not occur.  So maybe this bug is only a problem on sites that get squished together on narrow screens (hence maybe not so much newer devices).

There seems to be a very specific set of conditions under which this bug is consistently reproducible.
45 is already shipped so we can't fix it there. Since you can reliably reproduce the problem I think the next step here is to make a build with additional logging (based on Aurora or Beta, since they still use the Java PZC code), and have you reproduce the problem with that build to get the logging. It'll probably take a few builds to figure out exactly what's going on.
status-firefox45: affected → wontfix
dontarius, can you install the APK at [1] and get a logcat while reproducing the problem? The APK will install as "Aurora", and you can verify it is the right build by going to about:buildconfig and checking that the "Built from" URL is "https://hg.mozilla.org/try/rev/5ca9a25ccc5a216c9a20e7316df2963ec43707db".

If you're not familiar with the adb command-line tool, you can get a logcat using the LogView add-on at [2] - it should work fine for Android 4.4.4 although it has issues with Android 5.0+. You can post the logs to pastebin directly from the addon (Menu > Tools > Logs) or via about:logs. In particular the logcat should contain various lines that include "Bug1259735" in the output, which should provide me with some useful information as to what is going on.

Thanks!

[1] http://archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-5ca9a25ccc5a216c9a20e7316df2963ec43707db/try-android-api-15/fennec-47.0a2.en-US.android-arm.apk
[2] https://addons.mozilla.org/en-US/android/addon/logview/
Flags: needinfo?(dontarius)
Hi, I was able to reproduce with the build and posted the log at:

https://pastebin.mozilla.org/8866541

Let me know if you need me to do something else.
Flags: needinfo?(dontarius)
Had a quick look at the log.  Not really sure what everything means, but lines 216 to 219 might be interesting:

04-05 22:25:34.695 14453 14481 D GeckoBrowser: Bug1259735: browser.js MozScrolledAreaChanged to 396.79998779296875x3180.333251953125
04-05 22:25:34.705 14453 14481 D Bug1259735: GLC::handleViewportMessage(PAGE_SIZE) 397.0x541.0
04-05 22:25:34.735 14453 14481 D GeckoBrowser: Bug1259735: browser.js MozScrolledAreaChanged to 396.79998779296875x3179.333251953125
04-05 22:25:34.735 14453 14481 D Bug1259735: GLC::handleViewportMessage(PAGE_SIZE) 397.0x541.0
Thanks! That's very useful. It looks like when the MozScrolledAreaChanged event is received [1] the page is already fully sized to 396.79998 x 3180.33325 but when it calls the sendViewportUpdate on the next line, it sends a page size of 397.0x541.0 to the Java code. The most likely explanation for this is that it's hitting the condition at [2].

The first part of that condition (readyState === "complete") is expected to be false, because the page is still loading. However, the second part of the condition (pageLargerThanScreen) should be true, because the page size is larger than the screen. Except it's not, because the screen is 397 wide and the page is 396.79998, so it's off by a fraction of a pixel. The Math.floor on the previous line is not sufficient to account for this, I think we need to Math.ceiling the page size as well.

I'll push another patch with a fix as well as more logging to confirm.

[1] https://hg.mozilla.org/try/file/5ca9a25ccc5a/mobile/android/chrome/content/browser.js#l4203
[2] https://hg.mozilla.org/try/file/5ca9a25ccc5a/mobile/android/chrome/content/browser.js#l3814
Assignee: nobody → bugmail.mozilla
Here is the build: http://archive.mozilla.org/pub/mobile/try-builds/kgupta@mozilla.com-2fad70cccb3e8eae919963aaa41747ad325128b3/try-android-api-15/fennec-47.0a2.en-US.android-arm.apk

Please do the same with this build as you did with the old one. I expect that you will *not* see the bug, but in cases where you might have seen the bug previously you should see output that includes "Bug1259735: BUG FIXED!!!" in the log. I suggest loading a few pages and checking the log to see if this output appears. If the output appears, or if you see the original bug, please attach the log. Thanks!
Flags: needinfo?(kbrosnan)
I don't see the bug in this new build.  Thanks very much!! The log is at:

https://pastebin.mozilla.org/8866673

(for some reason it seems only part of the log went up, let me know if you need more)
Perfect, thanks! I'll get the final patch up in a sec.
Could you please send this patch to the beta branch ? I experience this bug every day and it's quite annoying, I don't really want to switch to nightly or wait 6+ weeks.
Yes, I intend to request uplift to beta 46 once it is reviewed and landed. I agree that this is a very annoying issue that we should fix as soon as possible.
Duplicate of this bug: 1263138
Comment on attachment 8739001 [details]
MozReview Request: Bug 1259735 - Guard against a rounding error that could lead to Java having a stale page size, rendering the page unscrollable. r?snorp

https://reviewboard.mozilla.org/r/45015/#review42163
Attachment #8739001 - Flags: review?(snorp) → review+
https://hg.mozilla.org/mozilla-central/rev/a68f4eb45a85
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Still need to be uplifted to beta.
Comment on attachment 8739001 [details]
MozReview Request: Bug 1259735 - Guard against a rounding error that could lead to Java having a stale page size, rendering the page unscrollable. r?snorp

Approval Request Comment
[Feature/regressing bug #]: long-standing race condition which seems to have become more frequent recently. Note that this does *not* affect Nightly, because the C++ APZ code that is enabled on Nightly uses a different code path. However it affects aurora 47, beta 46, and earlier versions.
[User impact if declined]: pages that should be scrollable are not scrollable until the user switches tabs or rotates the devices. it's a pretty frustrating experience.
[Describe test coverage new/current, TreeHerder]: Tested by creating an Aurora build with this patch and having users run it to verify.
[Risks and why]: very low-risk, it's a small rounding error fix.
[String/UUID change made/needed]: none
Attachment #8739001 - Flags: approval-mozilla-beta?
Attachment #8739001 - Flags: approval-mozilla-aurora?
Comment on attachment 8739001 [details]
MozReview Request: Bug 1259735 - Guard against a rounding error that could lead to Java having a stale page size, rendering the page unscrollable. r?snorp

Regression seems pretty severe and the fix was verified, Aurora47+
Attachment #8739001 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8739001 [details]
MozReview Request: Bug 1259735 - Guard against a rounding error that could lead to Java having a stale page size, rendering the page unscrollable. r?snorp

Scrolling is important! Please uplift to beta.
Attachment #8739001 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Tested using:
Device: Huawei Honor (Android 5.1.1)
Build: Firefox for Android Beta - 47.0b6
Aurora - 48.0a2(2016-05-15)
Nightly - 49.0a1 (2016-05-15)
I'll mark this bug as verified fixed since it is verified on all versions.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.