Closed Bug 753742 Opened 8 years ago Closed 7 years ago

Content at hotmail.com is overlapped when panning

Categories

(Firefox for Android :: General, defect)

14 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- betaN+

People

(Reporter: AdrianT, Assigned: cwiiis)

References

Details

(Whiteboard: [testday-20120518])

Attachments

(3 files, 2 obsolete files)

Nightly/15.0a1 (2012-05-09) / Firefox Native 14.0b1 build 2
Device: HTC Desire
OS: Android 2.2.2

Steps to reproduce:
1. Go to Hotmail.com and Sign In with a valid user account. Use the account provided in the Note section if needed.
2. From the top menu choose "Hotmail" to view your inbox.
3. Pan around the page.

Expected results:
Content remains correctly displayed.

Actual results:
Content is overlapped with other parts of the page which seem to be low-resolution screenshots - please see the attached screenshot.
On Nightly 15.0a1 2012-05-09 there are white areas which are white covering the content which may be due to Bug 752910 - low res checkerboard is broken. 

Notes:
Test account credentials:
user: atestsv@hotmail.com
password: firsttest
Attached file logs
This looks like some incorrect invalidation or painting issue. Thanks for the test-account.
Whiteboard: [14.0b1]
blocking-fennec1.0: --- → ?
Still happens on nightly 5/10/2012 multilocale.  Samsung Galaxy S II
This looks exactly like bug 751690 which should be fixed. Can we re-test the STR in bug 751690 too and see if the patches just didn't fix the bug?
Assignee: nobody → chrislord.net
blocking-fennec1.0: ? → betaN+
This is still an issue on Nightly 15.0a1 2012-05-14. Tested on HTC Desire Z running Android 2.3.3.
This is producing a similar effect for me as bug 749741 and bug 750211 - it kind of looks like we're getting the viewport coordinates of the rendered document wrong somehow...

As you scroll, when it re-renders, it appears as though the re-rendered content has scrolled twice the distance it should - You can see this by just scrolling the list of mails slowly downwards (moving slowly allows it to re-render quickly enough for async scrolling to not be too visible), and seeing how the position under your finger moves.

Interestingly, events are still delivered to the right area of the page.
Had a look via the inspector, this site is actually a full-screen iframe.

What could be happening here is that the root scrollable frame code is finding the iframe, but the code we use to actually scroll will be trying to scroll the document body node.

In fact, I'm pretty certain this is what's happening - we're scrolling the iframe fine - but the document isn't scrolling because it can't... The compositor picks up the iframe and thinks it did scroll, when in fact, it didn't, so we end up with the scroll applied twice - once from the iframe scrolling, then again by the async scroll code. Or something along these lines at least.

Somehow, we need to have GetPrimaryScrollableLayer not be tricked by a scrollable iframe occupying an entire unscrollable document. I'll know more when this build finishes...
So I was right, but the fix isn't so simple - Ideally, we could just use FrameMetrics::IsRootScrollable instead of IsScrollable in CompositorParent::GetPrimaryScrollableLayer, but because our 'root scrollable' is not at the root of frame tree, it doesn't get marked as such.

Perhaps we want to change this, or perhaps we could add another ID for nodes with a display-port set, which we could use to fetch the layer representing our 'root scrollable'. I'm going to have a shot at doing the latter, as it sounds easier to me.

Ccing roc (sorry!) for comment, as this is related to layout.
Status: NEW → ASSIGNED
Oh, of course, FrameMetrics already has the display-port set, so presumably we could just look for the first layer with a display-port set? Trying this out.
Well, that does get us the same behaviour as looking for the first scrollable layer (and is perhaps more reliably what we want?), but this bug still manifests.

I'll start looking at the dumped display list and generated layers, but I'm starting to think that the iframe layer is being optimised into the root layer... Or that might just be nonsense. Will report back.
We could just change the meaning of the root scroll ID to be for the root content document scroll frame. This matches was e10s Fennec does, and get us what we want for Native Fennec. I don't think it would require much code change.
(In reply to Chris Lord [:cwiiis] from comment #10)
> I'll start looking at the dumped display list and generated layers, but I'm
> starting to think that the iframe layer is being optimised into the root
> layer... Or that might just be nonsense. Will report back.

If the parent document (the one that contains the iframe) is overflow hidden then this code http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2278 means we won't create a scroll layer display item and consequently won't create a scroll layer.
(In reply to Timothy Nikkel (:tn) from comment #11)
> We could just change the meaning of the root scroll ID to be for the root
> content document scroll frame. This matches was e10s Fennec does, and get us
> what we want for Native Fennec. I don't think it would require much code
> change.

This sounds good to me.

(In reply to Timothy Nikkel (:tn) from comment #12)
> (In reply to Chris Lord [:cwiiis] from comment #10)
> > I'll start looking at the dumped display list and generated layers, but I'm
> > starting to think that the iframe layer is being optimised into the root
> > layer... Or that might just be nonsense. Will report back.
> 
> If the parent document (the one that contains the iframe) is overflow hidden
> then this code
> http://mxr.mozilla.org/mozilla-central/source/layout/generic/
> nsGfxScrollFrame.cpp#2278 means we won't create a scroll layer display item
> and consequently won't create a scroll layer.

Thanks, this has enabled me to boil it down into a simpler test-case: http://chrislord.net/files/mozilla/iframe-test.html - you need to zoom in a bit after it loads, but after doing so, scrolling down exhibits the same behaviour as hotmail.
(In reply to Timothy Nikkel (:tn) from comment #12)
> (In reply to Chris Lord [:cwiiis] from comment #10)
> If the parent document (the one that contains the iframe) is overflow hidden
> then this code
> http://mxr.mozilla.org/mozilla-central/source/layout/generic/
> nsGfxScrollFrame.cpp#2278 means we won't create a scroll layer display item
> and consequently won't create a scroll layer.

So one thing that obviously fixes this is to force layer building on any scroll frame that has a display-port set. Would this be acceptable?
(In reply to Chris Lord [:cwiiis] from comment #14)
> So one thing that obviously fixes this is to force layer building on any
> scroll frame that has a display-port set. Would this be acceptable?

I would say forcing a scroll layer on the root scroll frame (mIsRoot) of the root content document (mOuter->PresContext()->IsRootContentDocument()) would work.
Patch does as suggested in comment #15.
Attachment #624103 - Flags: review?(tnikkel)
Duplicate of this bug: 749741
Duplicate of this bug: 750211
Whiteboard: [14.0b1] → [has proposed patch]
Whiteboard: [has proposed patch] → [has proposed patch][autoland-try]
Whiteboard: [has proposed patch][autoland-try] → [has proposed patch][autoland-in-queue]
Comment on attachment 624103 [details] [diff] [review]
Force layer building on root scroll frames of root content documents

Oh, I forgot one thing, we don't want to create scroll layers on desktop. I think this will create scroll layers on desktop.

Can we just ignore the overflow hidden check if we are usingDisplayport (ie always create a scroll layer if we are usingDisplayport)? I think we can.
Force layer building on scroll-frames with displayports, as suggested in comment #14 and comment #19.
Attachment #624103 - Attachment is obsolete: true
Attachment #624103 - Flags: review?(tnikkel)
Attachment #624334 - Flags: review?(tnikkel)
Comment on attachment 624334 [details] [diff] [review]
Force layer building on scroll frames with displayports

This looks like the same patch. Wrong file or no qref or something?
Upload the correct patch (sorry, VM fail)
Attachment #624334 - Attachment is obsolete: true
Attachment #624334 - Flags: review?(tnikkel)
Attachment #624689 - Flags: review?(tnikkel)
Comment on attachment 624689 [details] [diff] [review]
Force layer building on scroll frames with displayports

Change the comment to say something like "so CompositorParent can always find the scrollable layer for the root content document".
Attachment #624689 - Flags: review?(tnikkel) → review+
Pushed to inbound with fixed comment: http://hg.mozilla.org/integration/mozilla-inbound/rev/7d409147c4fd
Target Milestone: --- → Firefox 15
Attachment #624689 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7d409147c4fd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [has proposed patch][autoland-in-queue]
Panning hotmail no longer displays the content overlapped on Nightly 15.0a1 2012-05-18 however long HTML emails ( Newsletters for e.g) are cut off at the edge of the viewport. Bug 756453 - Long HTML emails are cut off at the bottom at hotmail.com - covers the new issue.

Verified fix on:
Build: Nightly 15.0a1 2012-05-17
Device: HTC Desire Z
OS: Android 2.3.3
Status: RESOLVED → VERIFIED
Given the number of duplicates, should this have some sort of test?
Flags: in-testsuite- → in-testsuite?
No longer blocks: 754598
Duplicate of this bug: 754598
Attachment #624689 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verify that this bug is fixed on nightly (v15 - 18 May).

The bug still affects beta (v14.0b2)

Tested on sgs2 cyanogenmod 9 (ics)

Tested url: Hotmail.com after login
Whiteboard: [testday-20120518]
Status: VERIFIED → RESOLVED
Closed: 7 years ago7 years ago
Pushed to Aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/2a3a10b7fc25
Target Milestone: Firefox 15 → Firefox 14
The Nightly version where it landed is 15.
Target Milestone: Firefox 14 → Firefox 15
Status: RESOLVED → VERIFIED
Comment on attachment 624689 [details] [diff] [review]
Force layer building on scroll frames with displayports

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Hotmail and other sites will show the bug described.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Attachment #624689 - Flags: approval-mozilla-beta?
This bug may show the first time w/ Adreno devices when landing on hotmail, it will correct itself after some time (several seconds) and does not crash.  Probably best to log as a separate bug as it seems only to affect adreno devices.

On LG Revolution, it will show a caught exception:
05-22 08:52:42.895: W/System.err(9221): java.lang.IndexOutOfBoundsException: setSpan (0 ... 11340) ends beyond length 22
05-22 08:52:42.895: W/System.err(9221): 	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:948)
05-22 08:52:42.895: W/System.err(9221): 	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:527)
05-22 08:52:42.905: W/System.err(9221): 	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:519)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoInputConnection.setEditable(GeckoInputConnection.java:1042)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoInputConnection.notifyTextChange(GeckoInputConnection.java:457)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoInputConnection.notifyIMEChange(GeckoInputConnection.java:978)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoAppShell.notifyIMEChange(GeckoAppShell.java:527)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:472)
05-22 08:52:42.905: W/System.err(9221): 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:82)

On HTC Desire HD:
05-22 08:23:27.131: E/GeckoConsole(4187): [JavaScript Error: "NS_ERROR_INVALID_POINTER: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIEventListenerService.getListenerInfoFor]" {file: "chrome://browser/content/browser.js" line: 3041}]
05-22 08:23:27.161: D/PowerManagerService(1377): New lightsensor value:160, lcdValue:163
05-22 08:23:27.161: D/PowerManagerService(1377): lightSensorChangedLocked, buttonValue >= 0, mPowerState = 3
05-22 08:23:27.732: E/GeckoConsole(4187): [JavaScript Warning: "Empty string passed to getElementById()." {file: "http://js.wlxrs.com/mxFacnptZSBDQMbNrEZZ7w/wlive.js" line: 1}]
05-22 08:23:27.982: E/GeckoConsole(4187): [JavaScript Error: "[Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIEventListenerService.getListenerInfoFor]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: chrome://browser/content/browser.js :: _hasMouseListener :: line 3041"  data: no]" {file: "chrome://browser/content/browser.js" line: 2666}]
Created bug 757472; leaving this bug closed/verified.
Comment on attachment 624689 [details] [diff] [review]
Force layer building on scroll frames with displayports

[Triage Comment]
It's not clear why 13 would need a FN fix since we're not shipping the product off that version.
Attachment #624689 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Verified fixed on Aurora 14.0a2 (2012-05-24)
                  Beta 14.0b3
Samsung Galaxy SII (2.3.4)
Duplicate of this bug: 750645
You need to log in before you can comment on or make changes to this bug.