Closed Bug 818575 Opened 12 years ago Closed 11 years ago

Content becomes white after tapping into a text area from a shortcut made on the homepage and scrolling happens at double speed

Categories

(Firefox OS Graveyard :: General, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed)

VERIFIED FIXED
B2G C4 (2jan on)
blocking-basecamp +
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed

People

(Reporter: nhirata, Assigned: jrmuizel)

References

Details

(Keywords: regression, Whiteboard: [b2g-gfx])

Attachments

(12 files, 5 obsolete files)

19.04 KB, image/png
Details
17.42 KB, image/png
Details
29.53 KB, image/png
Details
190 bytes, text/html
Details
835 bytes, patch
Details | Diff | Splinter Review
2.34 KB, patch
cjones
: review-
Details | Diff | Splinter Review
6.63 KB, text/html
Details
15.70 KB, patch
cjones
: review+
roc
: review+
Details | Diff | Splinter Review
16.01 KB, patch
nrc
: review+
Details | Diff | Splinter Review
4.10 KB, patch
cjones
: review+
Details | Diff | Splinter Review
671 bytes, patch
Details | Diff | Splinter Review
15.84 KB, patch
mattwoodrow
: review+
Details | Diff | Splinter Review
## Environment :
Unagi phone, build 2012-12-04

## Repro :
1. launch browser
2. go to http://people.mozilla.com/~nhirata/html_tp/formsninput.html
3. select the star -> add to homescreen
4. hit home
5. select the new icon
6. tap into the bottom text area

## Expected :
1. keyboard will come up and you can see part of the text area

## Actual :
1. after the keyboard comes up, the content area is completely white; scrolling upwards will show part of the rendering.

## Note :
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Priority: -- → P2
I noticed that I can reproduce this with the persona login found in the marketplace settings.
I can't reproduce in 12/9 unagi in homescreen bookmark nor persona login.
(In reply to Dietrich Ayala (:dietrich) from comment #4)
> I can't reproduce in 12/9 unagi in homescreen bookmark nor persona login.

100% reproduction on my end with a persona login with a trusted UI.
Blocks: 820059
(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Dietrich Ayala (:dietrich) from comment #4)
> > I can't reproduce in 12/9 unagi in homescreen bookmark nor persona login.
> 
> 100% reproduction on my end with a persona login with a trusted UI.

Unagi? Which build date? And is trusted UI the default now?
(In reply to Dietrich Ayala (:dietrich) from comment #8)
> (In reply to Jason Smith [:jsmith] from comment #7)
> > (In reply to Dietrich Ayala (:dietrich) from comment #4)
> > > I can't reproduce in 12/9 unagi in homescreen bookmark nor persona login.
> > 
> > 100% reproduction on my end with a persona login with a trusted UI.
> 
> Unagi? Which build date? And is trusted UI the default now?

12/11/2012 beta build. Unagi. Trusted UI with persona is not on by default right now, but we are waiting on this bug to get fixed to land the pref to turn it on.
Jed, are you the right person to dig into this? Or give some context about where the problem is, how a developer would get into the scenario to reproduce it, etc?

Naoki, you were seeing this *without* trusted UI turned on?
Blocks: 811528
(In reply to Dietrich Ayala (:dietrich) from comment #10)
> Jed, are you the right person to dig into this? Or give some context about
> where the problem is, how a developer would get into the scenario to
> reproduce it, etc?

Jed and I were actually digging into this for a good amount of the day. He was able to reproduce it consistently with the trusted UI as well. 

STR:

1. You can use https://bugzilla.mozilla.org/show_bug.cgi?id=811528#c3 to figure out how to pref on identity for trusted UI. 
2. Launch the marketplace app
3. Click login
4. Select the enter your address field

Result - you'll get the white blob of doom this bug talks about

> 
> Naoki, you were seeing this *without* trusted UI turned on?

His STR imply he's seeing it without the trusted UI (his screenshots indicate that). Rudy commented on a related bug mentioning that the white blob issue we are seeing is not a keyboard bug either.

Hmm...Naoki did mention this could potentially be a gecko bug.

Chris Jones - Any ideas?
Flags: needinfo?(jones.chris.g)
Let's send this over to gecko triage actually and see if Chris or Andrew can help out here.

I have a strong hunch this is a gecko issue in disguise.

And flipping a bunch of keywords also along with higher priority renom:

1. This blocks the ability to turn on identity with trusted UI by default - this will be a smoketest when prefed on, so this inherently is a smoketest blocker
2. It's definitely a regression - I wasn't seeing this the last time I tested trusted UI with identity
3. A regression window might help here
blocking-basecamp: + → ?
Component: Gaia::System → General
Flags: needinfo?(overholt)
Priority: P2 → --
I wonder if this is a dupe of bug 814252...
Whiteboard: DUPE of bug 814252
Whiteboard: DUPE of bug 814252 → DUPE of bug 814252?
(In reply to Jason Smith [:jsmith] from comment #13)
> I wonder if this is a dupe of bug 814252...
Nop, I can reproduce this bug with and without the patch of bug 814252.
Whiteboard: DUPE of bug 814252?
In the STR provided by nhirata, if you focus on other input field and try to scroll up, you will found the white area became larger. Looks like the repaint region is also shifting while you scroll.
I can reproduce using STR in comment 0 100% in a debug build.  I don't have any guesses for what's happening here though.  We should see which elements are scrolling here; where the scroll requests are being made.

Shih-Chiang, want to take a look? :)  You were looking at this code recently ;).
Assignee: nobody → schien
Flags: needinfo?(jones.chris.g) → needinfo?
Flags: needinfo?(overholt)
Flags: needinfo?
I found that reverting the gaia modification in bug 808724 made this problem disappear, but I'm still investigating the root cause.
Component: General → Gaia
Triage: BB+, P2 - regression, C3, remove smoketest blocker
blocking-basecamp: ? → +
Keywords: smoketest
Priority: -- → P2
Target Milestone: --- → B2G C3 (12dec-1jan)
(In reply to Joe Cheng from comment #18)
> Triage: BB+, P2 - regression, C3, remove smoketest blocker

Disagree. Turning the trusted UI on with persona will cause this to show up persistently in a smoke test. And this intends to become a smoketest once we flip the pref on.
blocking-basecamp: + → ?
Keywords: smoketest
Priority: P2 → --
Putting the flags back where they were. If the smoketest fails because of this, that's ok as long as we know, and the issue is owned. We need to get this fixed, and soon for exactly those same reasons.

I've bumped up priority so that this gets fixed sooner rather than later, due to ID/Payments dependency.
blocking-basecamp: ? → +
Priority: -- → P1
I see this extra whitespace/rendering problems in Marketplace as well, after logging in with Persona for the first time (and entering credentials through the keyboard). 12/12/12 nightly build. Adding a screenshot.
Add Walter for providing helps from QA
QA Contact: wachen
(In reply to Al Tsai [:atsai] from comment #23)
> Add Walter for providing helps from QA

There's already two ways to reproduce this bug. No need more QA involvement.
QA Contact: wachen
Oh, I see why you did it with the regression window. Never mind...
QA Contact: wachen
Update my observation:
1. This issue will happen on the page created by window.open(). You can reproduce this issue via homescreen bookmark, everything.me, and window.open tests in UI Tests.
2. When you touch the white area, you can still get touch event targeting the element in this window. The location of these HTML elemets are correct but the repaint area is shifting with the same offset as the scrolling distance.
3. bug disappeared after disabling OOP for the <iframe mozbrowser> in window_manager.js.
4. The scrolling target is the window object in the <iframe mozbrowser>, which loads the URL given by window.open().

unassign myself because I'm not familiar with repaint issues.

@cjones, can you find someone to take further investigation on this issue?
Assignee: schien → nobody
Component: Gaia → General
Flags: needinfo?(jones.chris.g)
CCing people who may be able to help or help find an owner.
Btw - With logging into persona with a trusted UI yesterday, I can no longer reproduce this issue.
Jason, please close if you can no longer reproduce.  Chris, it was suggested this might be an IME issue.  If it's still reproducible, can you help fix?
Assignee: nobody → cpeterson
Flags: needinfo?(jones.chris.g)
QA Contact: wachen → jsmith
(In reply to Andrew Overholt [:overholt] from comment #29)
> Jason, please close if you can no longer reproduce.  Chris, it was suggested
> this might be an IME issue.  If it's still reproducible, can you help fix?

Okay. I'll look into this tomorrow with Naoki's test case vs. my test case.
No longer blocks: 811528
(In reply to Jason Smith [:jsmith] from comment #30)
> (In reply to Andrew Overholt [:overholt] from comment #29)
> > Jason, please close if you can no longer reproduce.  Chris, it was suggested
> > this might be an IME issue.  If it's still reproducible, can you help fix?
> 
> Okay. I'll look into this tomorrow with Naoki's test case vs. my test case.

I can reproduce this on the 12/18 build with Naoki's test case.
Keywords: qawanted
Assignee: cpeterson → milan
Whiteboard: [b2g-gfx]
Jeff, could this be related to the other tap bug?
Assignee: milan → jmuizelaar
The height of the whitespace in the Marketplace screenshot looks suspiciously close to the height of the keyboard. Perhaps some scroll code is omitting or double-counting the keyboard height?
it sounds like it's most likely a problem with the compositor (CompositorParent) and ShadowLayers.
Adding a quick link for people to connect to it faster
http://0rz.tw/PmtuW
No longer blocks: 820059
adding benwa because of #34
Jeff, have you had a chance to look at this?
Not yet, no.
Btw - this will still reproduce if you complete a login into marketplace and try to scroll around marketplace. So as of 12/28 build, this is still reproducible.
This should be fixed "before" 1/1.
Please do help on looking into this issue.
Severity: normal → blocker
This may be fixed by bug 825692
(In reply to Jeff Muizelaar [:jrmuizel] from comment #42)
> This may be fixed by bug 825692
Nop, this bug is reproducible after applying the patch for bug 825692.
Target Milestone: B2G C3 (12dec-1jan) → B2G C4 (2jan on)
Attached file Reduced test case
This seems to happen when ever we bring up the keyboard and are scrolling when launching from a bookmark.
Would this be handled by the fix for bug 766066?
(In reply to Jet Villegas (:jet) from comment #45)
> Would this be handled by the fix for bug 766066?

I don't think so.
Summary: Content becomes white after tapping into a text area from a shortcut made on the homepage → Content becomes white after tapping into a text area from a shortcut made on the homepage and scrolling happens at double speed
It seems like the mEffectiveTransform on the Thebes layer ends up being too large in the compositor process. I don't see a similar mEffectiveTransform in the content process.
It feels like somethings is going wrong in mozilla::layers::ThebesLayer::ComputeEffectiveTransforms

For example,
we have a LocalTransform() with a y translation of -88
and a aTransformToSurface with a y translation of -68.1999969

LocalTransform()
and aTransformToSurface both change during scrolling which accumulates the effect.
This patch fixes the issue, but is unlikely to be correct.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #48)
> LocalTransform()
> and aTransformToSurface both change during scrolling which accumulates the
> effect.

Obviously they shouldn't both be affected by scrolling.

What does the layer tree look like? Maybe the content process is setting a transform on the ThebesLayer to reflect that layer's scrolling, and RenderFrameParent's TransformShadowTree sets the same transform on the ThebesLayer's ContainerLayer parent which we use for scrolling?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #50)
> What does the layer tree look like? Maybe the content process is setting a
> transform on the ThebesLayer to reflect that layer's scrolling, and
> RenderFrameParent's TransformShadowTree sets the same transform on the
> ThebesLayer's ContainerLayer parent which we use for scrolling?

Note that this shouldn't happen since the intent is that the transform set by RenderFrameParent expresses just the difference between the "async scroll offset" (the up-to-date offset managed by the compositor) and the "content scroll offset" (the scroll offset used by the content process the last time it updated the layer tree).
Here's the layer tree before and after scrolling. I've removed most of the parts that remain the same.

Before:
OGLShadowRefLayer (0x11897a0a8) [[shadow-transform=[ 1 0; 0 1; 0 -51; ]] [id=2]
  OGLShadowContainerLayer (0x117b2c4a8) [[metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=232.000000) viewportScroll=(x=0.000000, y=71.000000) }]
    OGLShadowThebesLayer (0x117b2d4a8) [shadow-transform=[ 1 0; 0 1; 0 -71; ]] [shadow-visible=< (x=0, y=71, w=320, h=232); >] [transform=[ 1 0; 0 1; 0 -71; ]] [visible=< (x=0, y=71, w=320, h=232); >] [valid=< (x=0, y=71, w=320, h=232); >]

After:
OGLShadowRefLayer (0x11897a0a8) [[shadow-transform=[ 1 0; 0 1; 0 -33; ]] [id=2]
  OGLShadowContainerLayer (0x117b2c4a8) [[metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=232.000000) viewportScroll=(x=0.000000, y=53.000000) }]
    OGLShadowThebesLayer (0x117b2d4a8) [shadow-transform=[ 1 0; 0 1; 0 -53; ]] [shadow-visible=< (x=0, y=53, w=320, h=232); >] [transform=[ 1 0; 0 1; 0 -53; ]] [visible=< (x=0, y=53, w=320, h=232); >] [valid=< (x=0, y=53, w=320, h=232); >]
I don't understand why scrolling is setting a transform on the ShadowRefLayer there. That seems wrong. The ShadowRefLayer is the layer that contains the entire layer tree for another process, so it's like we're scrolling both in the Web page and in the application that contains the Web page.
Just to note: 
facebook e.me is being affected by this bug I believe...  Both in the login as well as typing in message thread.
I believe this is also regressed by bug 811950. By reverting 1310b5aab1f4 I could scroll the content normally.
Per comments above, this has been reproduced on builds from b2g18.  Bug 811950 hasn't been uplifted there.
It looks like the ShadowRefLayer is getting it's transform set in CompensateForContentScrollOffset() which does seem suspicious.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #58)
> It looks like the ShadowRefLayer is getting it's transform set in
> CompensateForContentScrollOffset() which does seem suspicious.

Unsurprisingly, doing nothing in this function fixes the problem.
Chris, care to comment on whether this function is doing what it is supposed to be doing?
Flags: needinfo?(jones.chris.g)
Yes, I believe so.  That helper is trying to move the remote subtree layer space into the space of its non-remote container.
Flags: needinfo?(jones.chris.g)
I believe there was a change made in service of dlbi that may have made this code not only obsolete, but incorrect.  If so, then removing CompensateForScrollOffset() entirely would be the right fix.
A thorough review would be appreciated as I'm not very familiar with how this is supposed to work.
Attachment #699262 - Flags: review?(roc)
Attachment #699262 - Flags: review?(jones.chris.g)
Comment on attachment 699262 [details] [diff] [review]
Remove CompensateForContentScrollOffset

OK, we still need to apply this offset.  Patch here breaks the following test
 1. cd gaia && make reset-gaia
 2. Open "UI Test" app
 3. Tap "window.open tests"
 4. Tap "window.open" button
 5. Focus "focus me!" input

Panning around shows that the content is being translated incorrectly.  (Not accounting for scroll offset.)  :(
Attachment #699262 - Flags: review?(jones.chris.g) → review-
It seems that this bug, and bug 827231 only occur when pages are opened indirectly (through window.open I believe).

It would be interesting to see the differences in Layer trees when viewing this testcase opened directly in the browser vs opening it via a homescreen icon.

Haven't been able to reproduce either of these locally, b2g isn't behaving at all.
During scroll with the UItest we're transforming the ThebesLayer and the ContainerLayer by the same amount in the opposite directions. Presumably the Compensate function adds an additional transform to the RefLayer to make up for this.
Attachment #699883 - Attachment is patch: false
Attachment #699883 - Attachment mime type: text/plain → text/html
(In reply to Matt Woodrow (:mattwoodrow) from comment #65)
> It seems that this bug, and bug 827231 only occur when pages are opened
> indirectly (through window.open I believe).
> 
> It would be interesting to see the differences in Layer trees when viewing
> this testcase opened directly in the browser vs opening it via a homescreen
> icon.

It seems like the AsyncPanZoomController is only being used when viewing in browser. The rest of the time we take a different scrolling path.
So it looks like the problem here is actually that we're not adjusting the transform on the ContainerLayer underneath the RefLayer. This happening because the 'metrics' that TransformShadowTree gets claim that the scroll offset is 0
So I think I know what's going wrong. We have two RefLayers and each thinks it's scrollable. TransformShadowTree then picks the wrong one to transform.
It appears that we have multiple scrollable layers here, and GetPrimaryScrollableLayer (called from TransformShadowTree) is returning the 'wrong' one. I say 'wrong', because it's not clear to me why we would only want to apply this algorithm to only one.

Layer tree dump for the broken case: http://pastebin.mozilla.org/2048410

Note that OGLShadowContainerLayer (0x1157d70a8) and OGLShadowContainerLayer (0x11abdcca8) are both scrollable layers, GetPrimaryScrollableLayer is returning the former, yet actual scrolling is happening on the latter.

My patch changes the implementation of GetPrimaryScrollableLayer to walk backwards over the child layers so that we end up picking the top-most one. It fixes this test case at least, but I'd need to know more about what we're trying to achieve here before saying that it's actually correct.
It seems to me like we should probably Transform all of the scrollable layers.
Attachment #700134 - Attachment is obsolete: true
Attachment #700177 - Flags: review?(jones.chris.g)
Comment on attachment 700177 [details] [diff] [review]
Make TransformShadowTree adjust all scrollable layers

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

Looks good to me. I think you should land this. (Don't forget to fix the checkin comment.)

::: gfx/layers/Layers.cpp
@@ +239,5 @@
> +  nsTArray<Layer*> queue;
> +  queue.AppendElement(mRoot);
> +  while (queue.Length()) {
> +    ContainerLayer* containerLayer = queue[0]->AsContainerLayer();
> +    queue.RemoveElementAt(0);

This is really inefficient, O(N^2). You should remove the last element of the queue array. (Yes, I know you just copied it.)

::: gfx/layers/Layers.h
@@ +285,5 @@
>     */
>    Layer* GetPrimaryScrollableLayer();
>  
>    /**
> +   * Returns a list of all scrollable layers.

"all descendant layers for which GetFrameMetrics().IsScrollable() is true"
Attachment #700177 - Flags: review+
Fixed review comments, carrying forward r=roc
Attachment #700244 - Flags: review+
Attachment #699262 - Flags: review?(roc)
just rebased the v2 patch, carrying r=roc
Attachment #700244 - Attachment is obsolete: true
Attachment #700289 - Flags: review+
Attached patch patch rebased for b2g18 (obsolete) — Splinter Review
patch for b2g18, I can't seem to get it on to my phone to test it though
Attachment #700295 - Flags: review+
Attachment #700177 - Flags: review?(jones.chris.g) → review+
We seem to be crashing in the compositing thread while drawing the scrollbars (  mWidget->DrawWindowOverlay(this, rect);)
I imagine that is because we're now calling SyncViewportInfo more than once.
Attached patch This fix the crash (obsolete) — Splinter Review
This an untested patch that I expect will fix the crash. It's not great looking and does make me wonder if there will be other bad interactions caused by this code on Android.
Attached patch fixup patchSplinter Review
tested and works now on Fennec
Attachment #700389 - Attachment is obsolete: true
Attachment #700428 - Flags: review?(jones.chris.g)
Attachment #700428 - Flags: review?(jones.chris.g) → review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #83)
> Created attachment 700389 [details] [diff] [review]
> Something that has a better chance of building
> 
> https://tbpl.mozilla.org/?tree=Try&rev=96c44135f945

This doesn't seem to have fixed the problem
I haven't been able to reproduce the crashes locally yet, but maybe this will work better.
Trying swapping width/height back to the confused value:
https://tbpl.mozilla.org/?tree=Try&rev=e243e07e12e4
Patch that landed on inbound, rebased to b2g18
Attachment #700295 - Attachment is obsolete: true
Attachment #700845 - Flags: review+
Verified fixed

Device: Unagi
Build Identifier: 20130113070202
Update channel: 
   nightly b2g18 (pvt server)  
   unagi.zip	13-Jan-2013 08:06 	100M	 
   https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-b2g18-unagi-eng/
Git commit info: 2013-11-13 15:15:51
Status: RESOLVED → VERIFIED
Device: Unagi
Build Identifier: 20130113070202

## Repro :
1. launch browser
2. go to http://www.soso.com
3. select the star -> add to homescreen
4. hit home
5. select the new icon
6. tap on white area to make keyborad comes down.

## Expected :
1. keyboard will comes down and you still can see the content.

## Actual :
1. after the keyboard comes down, the content area is completely white; scrolling can show the content.

Youtube: http://www.youtube.com/watch?v=b-25bNnvH5o

Reopen.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Please file a new bug for the issue you encountered and link it as a dependency on this bug. Lean towards not reopening - it makes tracking blockers a royal pain.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
No longer blocks: 830256
Depends on: 830256
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: