830256 - The content becomes white after keyboard down from bookmark of homescreen.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Askeing Yen[:askeing][:fyen]]

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

Comment 1

[JP Rosevear [:jpr]]

Jeff, any relation to bug 818575 ?

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Nope. This is something else.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I can reproduce this on a B2G OS X build.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

This seems to happen for any input box: http://people.mozilla.org/~jmuizelaar/bugs/text.html

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Broken/White:

OGLShadowContainerLayer (0x117c580a8) [shadow-visible=< (x=0, y=142, w=320, h=223); >] [visible=< (x=0, y=142, w=320, h=223); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=365.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=142.000000, w=320.000000, h=223.000000) scrollId=1 }]
  OGLShadowThebesLayer (0x117c58ca8) [shadow-visible=< (x=0, y=142, w=320, h=223); >] [visible=< (x=0, y=142, w=320, h=223); >] [opaqueContent] [valid=< (x=0, y=142, w=320, h=223); >]

Fixed:
OGLShadowContainerLayer (0x117c580a8) [shadow-visible=< (x=0, y=0, w=320, h=365); >] [visible=< (x=0, y=0, w=320, h=365); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=365.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=320.000000, h=365.000000) scrollId=1 }]
  OGLShadowThebesLayer (0x117c58ca8) [shadow-visible=< (x=0, y=0, w=320, h=365); >] [visible=< (x=0, y=0, w=320, h=365); >] [opaqueContent] [valid=< (x=0, y=0, w=320, h=365); >]

The visible area for the content layer is wrong.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

I expect the bug is in CalculatePendingDisplayPort

Comment 8

[Jeff Muizelaar [:jrmuizel]]

We have an mScrollableRect with a height of 223 and we sort of clamp the display port to that. The problem is that the code expects mScrollableRect to be larger than than the compositionRect, but when the page is resized when the keyboard is up the scrollableRect will shrink to the new size.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Attachment: Ensure that scrollable rect as at least as big as the drawable area

Ensure that scrollable rect as at least as big as the drawable area. Otherwise we end up clamping the displayport to a smaller size than we should be.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 702107 [details] [diff] [review]
Ensure that scrollable rect as at least as big as the drawable area

This patch doesn't quite make sense to me.
 1. Where is the bad scrollableRect value coming from?
 2. If this happens, why would we think a priori we need to shift content left/up instead of right/down?

Comment 11

[Jeff Muizelaar [:jrmuizel]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #10)
> Comment on attachment 702107 [details] [diff] [review]
> Ensure that scrollable rect as at least as big as the drawable area
> 
> This patch doesn't quite make sense to me.
>  1. Where is the bad scrollableRect value coming from?

The bad scroll rect comes from having resized the content area to make room for the keyboard. i.e. if the actual height of the page is shorter than the content area the scrollableRect will only be as large as the content area. When the keyboard goes away we still have the previous scrollable rect because the page has not yet changed size to fill up the new space.

>  2. If this happens, why would we think a priori we need to shift content
> left/up instead of right/down?

This was an arbitrary choice. The only important part for this patch is that the scrollable rect be at least as large and not start before 0,0.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 702107 [details] [diff] [review]
Ensure that scrollable rect as at least as big as the drawable area

This patch makes me very uncomfortable because it feels like we're papering over a more fundamental problem, but this is a pretty serious bug and maybe the best we can do at this point.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #12)
> Comment on attachment 702107 [details] [diff] [review]
> Ensure that scrollable rect as at least as big as the drawable area
> 
> This patch makes me very uncomfortable because it feels like we're papering
> over a more fundamental problem, but this is a pretty serious bug and maybe
> the best we can do at this point.

I guess the fundamental problem is that the scrollableRect depends on the size of the displayport/viewport and we don't get the new scrollableRect until after the TabChild has changed size. What kind of solution would you suggest?

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I don't understand why gecko isn't reflowing out to the widget dimensions.  I also don't understand why content was trying to repaint at the ~y offset of the keyboard (I think, per comment 6) instead of y=0.

Comment 15

[Jeff Muizelaar [:jrmuizel]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #14)
> I don't understand why gecko isn't reflowing out to the widget dimensions. 

We intersect the displayport with the scrollableRect which is smaller than the widget dimensions. Presumably, the smaller displayport dimensions override the widget dimensions.

> I also don't understand why content was trying to repaint at the ~y offset
> of the keyboard (I think, per comment 6) instead of y=0.

This happens because we adjust the scrollOffset.y to compositionBounds.width - scrollableRect.width, which is a negative value.

We then translate by -scrollOffset.y intersect with the scrollableRect and translate by scrollOffset.y. This moves the displayport to half way down the page.

Comment 16

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 702107 [details] [diff] [review]
Ensure that scrollable rect as at least as big as the drawable area

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

::: gfx/layers/ipc/AsyncPanZoomController.cpp
@@ +842,5 @@
> +  // Ensure the scrollableRect is at least as big as the compositionBounds
> +  // because the scrollableRect can be smaller if the content is not large
> +  if (scrollableRect.width < compositionBounds.width) {
> +      scrollableRect.x = NS_MAX(0.f,
> +                                scrollableRect.x - compositionBounds.width - scrollableRect.width);

Should be brackets around (compositionBounds.width - scrollableRect.width)

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Attachment: Ensure that scrollable rect as at least as big as the drawable area v2

Comment 18

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/339344803492

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/339344803492

Comment 21

[Julien Wajsberg [:julienw]]

TEF+ Bug 832222 was marked as duplicate of this bug, so this bug should be TEF+ and we should uplift this patch to b2g18's branches.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1e1ed4598a7f
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/785cac703746

Comment 23

[Jeni]

Verified issue fixed on Unagi 
Build ID 20130214070203
Kernel Dec 5
Gecko:http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d1288313218e
Gaia: 6544fdb8dddc56f1aefe94482402488c89eeec49