1162656 - [Browser][SHB] Browser menu does not fully fit in the screen in landscape mode with SHB

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Yeojin Chung [:YeojinC]]

Attachment: logcat_Landscape.txt

Description:
The menu for browser does not fully fit to the screen when the SHB is enabled. Bug 1160191 was fixed, but the issue still reproduces when the device is set in landscape mode.


Repro Steps:
1) Update a Flame to 20150507064907.
2) Enable SHB in Developer Settings.
3) Launch Browser and navigate to a webpage.
4) Select the menu icon (... in upper left).
5) Set the device in landscape mode.

Actual:
The menu does not fully fit, and the browser is showing on the right side of the screen.

Expected:
The menu fully fits to the screen.


Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150507064907
Gaia: 83b27f522642ea573c57e882657ab5c73d4b07f4
Gecko: 403e3c2380b5
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Repro frequency: 6/6
See attached: screenshot, logcat

Comment 1

[Yeojin Chung [:YeojinC]]

Attachment: landscape.png

Comment 2

[Yeojin Chung [:YeojinC]]

This issue also reproduces on Flame 2.2.

Result: The menu does not fully fit, and the browser is showing on the right side of the screen.

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150507002500
Gaia: 83a63e0e6fcc22c6a74b06ef77b88d5049719cad
Gecko: 118ddfc76b60
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 37.0 (2.2)
Firmware Verson: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8603236 [details] [review]
[gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master

Sorry, didn't see that it was already assigned (got pinged from bug 1160191).
Mind reviewing it then? :)

Comment 5

[Alberto Pastor [:albertopq]]

Comment on attachment 8603236 [details] [review]
[gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master

LGTM!

I was thinking, that this problem was already solved in other dialogs, and the reason is that those dialogs are on the #dialog_overlay. Wouldn't make sense to make the context_menu a SystemDialog, in order to be handled by the SystemDialogManager as well? This problem would be solved for free.

That said, this is a 2.2 blocker and your patch is the one that makes sense to be uplifted, so r+.

Thanks!

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to Alberto Pastor [:albertopq] from comment #5)
> Comment on attachment 8603236 [details] [review]
> [gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master
> 
> LGTM!

Thanks!

> 
> I was thinking, that this problem was already solved in other dialogs, and
> the reason is that those dialogs are on the #dialog_overlay. Wouldn't make
> sense to make the context_menu a SystemDialog, in order to be handled by the
> SystemDialogManager as well? This problem would be solved for free.

But at the system level we wouldn't be able to edge swipe while the dialog is shown right?
That's why we moved a bunch of dialogs inside the appWindow element IIRC.

Comment 7

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/b055b141694e1f00967b26e31ff2564b91455c24

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8603236 [details] [review]
[gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): follow up of bug 1160191
[User impact] if declined: See Description
[Testing completed]: alert/prompt/browser menu test with and without SHB in landscape and portrait this time
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Comment 9

[Shally Li（leave from Mozilla）]

Per Comment 7,this bug has been landed and fixed on master.

This bug has been verified as pass on latest Nightly build of Flame v3.0 by the STR in Comment 0.

Actual results: The menu page fully fits to the screen on the portrait mode and landscape mode.
See attachment: verified_v3.0.png
Reproduce rate: 0/6


Device: Flame v3.0 build(Pass)
Build ID               20150510010201
Gaia Revision          5b2a150f6f5d29bddfaac13fcbbf099376f2f275
Gaia Date              2015-05-09 12:34:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/77d92f6d7679
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150510.045933
Firmware Date          Sun May 10 04:59:44 EDT 2015
Bootloader             L1TC000118D0

------------------------------------------------------
Leaving "verifyme" for v2.2 uplift and verification.

Comment 10

[Shally Li（leave from Mozilla）]

Attachment: verified_v3.0.png

Comment 11

[Josh Cheng [:josh]]

(In reply to Etienne Segonzac (:etienne) from comment #8)
> Comment on attachment 8603236 [details] [review]
> [gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): follow up of bug 1160191
> [User impact] if declined: See Description
> [Testing completed]: alert/prompt/browser menu test with and without SHB in
> landscape and portrait this time
Hi Etienne,
Is it possible to add some unit test here? Thanks!

> [Risk to taking this patch] (and alternatives if risky): low
> [String changes made]: none

Comment 12

[Josh Cheng [:josh]]

Hi Lancy,
Can you check whether the issue also happen in 2.1?

Comment 13

[Etienne Segonzac (:etienne)]

(In reply to Josh Cheng [:josh] from comment #11)
> (In reply to Etienne Segonzac (:etienne) from comment #8)
> > Comment on attachment 8603236 [details] [review]
> > [gaia] etiennesegonzac:bug-1162656 > mozilla-b2g:master
> > 
> > [Approval Request Comment]
> > [Bug caused by] (feature/regressing bug #): follow up of bug 1160191
> > [User impact] if declined: See Description
> > [Testing completed]: alert/prompt/browser menu test with and without SHB in
> > landscape and portrait this time
> Hi Etienne,
> Is it possible to add some unit test here? Thanks!

Sadly no :/
It's a css change, the only meaningful way to test it would be with integration tests and we don't support landscape mode in integration tests.

Comment 14

[Yeojin Chung [:YeojinC]]

(In reply to Josh Cheng [:josh] from comment #12)
> Hi Lancy,
> Can you check whether the issue also happen in 2.1?

2.1 is unaffected. The menu fully fits to the screen.

Environmental Variables:
Device: Flame 2.1
BuildID: 20150509032506
Gaia: 3e7bd686ecd852f4dfa4605b45f558e6bd34f02a
Gecko: de4ebfdc7db6
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[helen（leave from Mozilla）]

This bug has been verified as pass on latest build of Flame v2.2 by the STR in Comment 0.

Results:The menu fully fits to the screen on the portrait mode and landscape mode.
Attachment: v2.2.jpg
Reproduce rate: 0/10

Device: Flame 2.2 build(Pass)
Build ID               20150513162505
Gaia Revision          aac58a063e3e6acae6ba77fe4cec224fb69450bc
Gaia Date              2015-05-13 12:59:48
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7a4f3cb5bf7b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150513.205814
Firmware Date          Wed May 13 20:58:25 EDT 2015
Bootloader             L1TC000118D0

Comment 17

[helen（leave from Mozilla）]

Attachment: v2.2.jpg