1119723 - [Flame][Homescreen]A partial facebook view overlaps on the top left corner of home screen.

Status: VERIFIED FIXED

Description

[SandKing]

Attachment: Bug log: logcat_0152.txt

[1.Description]:
[Flame][v2.1&v2.2][Homescreen]When you back to homescreen from facebook page(in Contacts) in landscape mode, there is a partial facebook view overlaps on the top left corner of home screen.
Found Time:01:52
Bug log:"logcat_0152.txt".
Bug video:"0152.MP4".

[2.Testing Steps]: 
1. Launch "Contacts".
2. Press "gear" icon.
3. Enable "Sync firends" item.
4. Rotate device to landscape mode in "facebook" page.
5. Press Home button.

For Flame v2.1, after above steps,need to add the following steps:
6. Launch Gallery, and open a picture.
7. Rotate to landscape mode again.
8. Tap the "Camera" icon to invoke Camera.
9. Press Home button.

[3.Expected Result]: 
4.The Home page should display completely.

[4.Actual Result]: 
4. A partial facebook view will overlap on the left top corner of home screen.

[5.Reproduction build]: 
Flame 2.1:
Gaia-Rev        ed2e278753e8c9301ba322dcf2c3591f5928408d
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/127a0ead5f83
Build-ID        20150108001214
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150108.040209
FW-Date         Thu Jan  8 04:02:21 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 build:
Gaia-Rev        d4dac29613076bdba3cb8adc217deadb08a2ac20
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/70de2960aa87
Build-ID        20150108010221
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150108.044406
FW-Date         Thu Jan  8 04:44:17 EST 2015
Bootloader      L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]:
Free Test

Comment 1

[SandKing]

Attachment: Bug video: 0152.MP4

Comment 2

[Mike Lien[:mlien]]

The problem is caused by sofware homwbutton, it won't have this problem if disable sofware homebutton.

Comment 3

[SandKing]

Hi Mike,
Don't enable "sofware home button", this issue still exists

Comment 4

[Mike Lien[:mlien]]

Hi Alive, not sure if this is belong to system. Could you help to check with it, thanks.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Bad :/

Could you help? A potential blocker.

Comment 6

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

The root cause is, when gaia gets orientationchange, it only resize the activeWindow instead of its childWindows. I think we should call resize as my commit.

waiting for test result.

Comment 7

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8550128 [details] [review]
PR to master

Hi Alive,
my proposal is to change _resize to resize. what do you think?

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8550128 [details] [review]
PR to master

I believe the orientationchange handler needs to be done for every appWindow instead of only active Appwindow. By calling this.resize() you will be early returned by bottom.shouldResize() and fail to resize the background appWindow.

Anyway, Etienne should review this.

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8550128 [details] [review]
PR to master

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #8)
> I believe the orientationchange handler needs to be done for every appWindow
> instead of only active Appwindow. By calling this.resize() you will be early
> returned by bottom.shouldResize() and fail to resize the background
> appWindow.

Indeed.

I think the core issue here is that the orientationchange event only gets broadcasted to launched apps not inline activities.

One fix would be to broadcast the event to `this.frontWindow` in the appWindow has one. What do you guys think?

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Etienne Segonzac (:etienne) from comment #9)
> Comment on attachment 8550128 [details] [review]
> PR to master
> 
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #8)
> > I believe the orientationchange handler needs to be done for every appWindow
> > instead of only active Appwindow. By calling this.resize() you will be early
> > returned by bottom.shouldResize() and fail to resize the background
> > appWindow.
> 
> Indeed.
> 
> I think the core issue here is that the orientationchange event only gets
> broadcasted to launched apps not inline activities.
> 
> One fix would be to broadcast the event to `this.frontWindow` in the
> appWindow has one. What do you guys think?

+1!

Comment 11

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8550128 [details] [review]
PR to master

Thanks for advice!
Now app would broadcast orientaitonchange to its frontWindow if app is active.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8550128 [details] [review]
PR to master

Let's see if there is side effect or not.

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Test pass, Thanks everyone.
master: https://github.com/mozilla-b2g/gaia/commit/05f8670c8d457f67c4202de4094e9ca9ea409bf4

Comment 14

[Coler]

This issue has been verified successfully on Flame master.

Flame master build:

Gaia-Rev        22230c9a340a0a2c3af73320cf47e1cb544517ad
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/600f44fd317c
Build-ID        20150226160308
Version         39.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150226.193049
FW-Date         Thu Feb 26 19:31:00 EST 2015
Bootloader      L1TC000118D0

Comment 15

[Eric Chang [:ericcc] [:echang]]

Set 2.2?, we have PR, and to make ux more friendly, could we uplift this to 2.2?

Comment 16

[bhavana bajaj [:bajaj]]

(In reply to Eric Chang [:ericcc] [:echang] from comment #15)
> Set 2.2?, we have PR, and to make ux more friendly, could we uplift this to
> 2.2?

Hmm, I don't want to block on this as this is not a recent regression and more of a polish. nevertheless I am happy to uplift this if George can request approval, that way if this has any fallouts we can immediately back this out without worrying about tracking the follow-ups. I'll make an exception to approve this non-blocking issue as the risk looks low and worth the polish.

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8550128 [details] [review]
PR to master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): None.
[User impact] if declined: User might see unexpected facebook screen when doing steps of comment 0. (see attachment 8546514 [details] )
[Testing completed]: Yes. it has covered manual test and unit test.
[Risk to taking this patch] (and alternatives if risky): None.
[String changes made]:

Comment 18

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/11fac297fa3448cadec1b555c11a0aa18033522e

Comment 19

[Elie]

This issue verified successfully on Flame 2.2
Flame 2.2:
Build ID               20150327162502
Gaia Revision          473cd63f53c855299b719285d9b95e3f2910782f
Gaia Date              2015-03-27 20:14:43
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/b358619def45
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Reproduce rate  0/5