1166075 - [aries] Full screen video permission overlay is not full screen in landscape mode

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. go to www.youtube.com
2. pick a video in portrait
3. play video
4. set to full screen
5. change to landscape

Expected: the overlay covers the whole screen
Actual: overlay doesn't cover the whole screen

Note: 
1. doesn't occur with geolocation when going to maps.google.com
2. aries only; doesn't happen with flame

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

[Blocking Requested - why for this release]:

Build ID               20150516172027
Gaia Revision          4c0f36e9dfe017bf2a698d416a57c8156b43383d
Gaia Date              2015-05-15 22:18:51
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2f6ea66057fe
Gecko Version          41.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150516.170603
Firmware Date          Sat May 16 17:06:10 UTC 2015
Bootloader             s1

Comment 2

[Gregor Wagner [:gwagner]]

Can we get a branch check and eventually a regression window?

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

I can reproduce this bug on latest 3.0, but we don't have information on what branches are valid on this device, nor do we have links to access them if we do support more than one branches on it. NI Naoki on this.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

It only happens on spark.  I think it's a device display size issue.

Finding correct revisions for Aries on TC is a pain.  We need bug 1133074 or some other routes to the spark build revs... pinging wcosta for further input.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

According to Naoki, spark only has master branch on it, so branch checking is not possible. Removing qawanted.

Note: Comment 3 was tested on a spark device, if I wasn't clear on that I apologize.

Comment 6

[Wander Lairson Costa]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #4)
> It only happens on spark.  I think it's a device display size issue.
> 
> Finding correct revisions for Aries on TC is a pain.  We need bug 1133074 or
> some other routes to the spark build revs... pinging wcosta for further
> input.

^jonasfj

Comment 7

[Gregor Wagner [:gwagner]]

(In reply to Pi Wei Cheng [:piwei] from comment #3)
> I can reproduce this bug on latest 3.0, but we don't have information on
> what branches are valid on this device, nor do we have links to access them
> if we do support more than one branches on it. NI Naoki on this.

How about 2.2 with SHB enabled on flame?

Comment 8

[Gregor Wagner [:gwagner]]

Please find assignee.

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Gregor Wagner [:gwagner] from comment #7)
> (In reply to Pi Wei Cheng [:piwei] from comment #3)
> > I can reproduce this bug on latest 3.0, but we don't have information on
> > what branches are valid on this device, nor do we have links to access them
> > if we do support more than one branches on it. NI Naoki on this.
> 
> How about 2.2 with SHB enabled on flame?

This issue reproduces on Flame 3.0, 2.2, and 2.1 with SHB enabled. The app permission overlay appears to not cover the whole screen when in landscape mode. Note that the reason why it appears it's not covering the whole screen is the SHB bar becomes transparent in landscape. I've made a video for this issue. The behavior seems half expected to me because the SHB bar stays transparent in landscape even without the overlay.

Video:
https://www.youtube.com/watch?v=oP0fass6XqU

Device: Flame
BuildID: 20150520010202
Gaia: 600fd8249960b8256af9de67d9171025bb9a3ff3
Gecko: ac277e615f8f
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Device: Flame
BuildID: 20150520002502
Gaia: 63e9eeec3032318f8a240f80b6a184fa4b50b6e1
Gecko: a89755309dea
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame
BuildID: 20150520001201
Gaia: c80865cb0bf73f1b97defbc646083b404feb3ac4
Gecko: bf8615dd05d6
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

SHB is not supported on v2.0 so not tested.

Comment 10

[Michael Henretty [:mikehenrty][:mhenretty]]

Ok, this looks like a bug to me, but definitely not a regression. We need some help from UX to decide if it's indeed a bug and what to do about it. Should we make the overlay cover the SHB button, or should we make the SHB container opaque for the fullscreen request case? My thought is the latter.

Rob, can you provide some UX input here?

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Ah ya.  I forgot to check SHB on the flame.  Thanks, gwagner and Pi Wei.

Comment 12

[Jonas Finnemann Jensen (:jonasfj)]

@nhirata
> Finding correct revisions for Aries on TC is a pain.  We need bug 1133074 or some other routes
> to the spark build revs... pinging wcosta for further input.
I'm not sure what you are looking for, or how you want to find it (is this route or UI).
Probably open a separate bug, with how you would like to discover things. E.g. what tasks you want,
and what strings you want to map to those task.
Note, TC index is essentially a key-value store, so it's can't track revision order, etc.

Comment 13

[Jacqueline Savory [:jsavory] UX]

Filling in for Rob here, I agree that this looks like a bug. Michael, I think that your second suggestion sounds good, the SHB should be opaque for landscape as well as portrait for full screen requests. This way we can keep the ability to tap home and its more consistent with portrait.

Comment 14

[Michael Henretty [:mikehenrty][:mhenretty]]

I'll take a look.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] mikehenrty:bug-1166075-permission-shb > mozilla-b2g:master

Comment 16

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8613128 [details] [review]
[gaia] mikehenrty:bug-1166075-permission-shb > mozilla-b2g:master

Alive, what do you think of this approach, ie. adding a class to screen to indicate we are showing a permission prompt, and therefore want to use the classic SHB? It's not ideal, but the only other way I could think of would be to have permission_manager use Service to request a display mode from the software button manager. Since most of the display modes for SHB are managed in CSS now, I feel like this approach would be adding a lot of complexity. Do you have any other ideas how we might tackle this issue?

Comment 17

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

Comment on attachment 8613128 [details] [review]
[gaia] mikehenrty:bug-1166075-permission-shb > mozilla-b2g:master

I dislike doing this, but whatever, if you agree to control the display mode from js is the right way to go, let's file followup to fix this properly.

* Note *
This is definitely hurting modularization if we want to make system to support NGA or something like NGA in the future. PermissionManager module cannot have any way to access #screen element in its scope.

Comment 18

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I saw this issue today while testing if bug 1167432 was still occurring.  It looks like the patch from comment 17 was never actually applied.  Naoki should this be passed on to someone else now?

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Hey Michael?  Did you want to redo the patch for NGA?  Or push the patch in?

Comment 20

[Michael Henretty [:mikehenrty][:mhenretty]]

It's on my todo list. I'll leave the ni? so that it keeps reminding me to finish :)

Comment 21

[Gregor Wagner [:gwagner]]

actually leave the NI :)

Comment 22

[Etienne Segonzac (:etienne)]

(In reply to Michael Henretty [:mhenretty][PTO_until_Oct] from comment #20)
> It's on my todo list. I'll leave the ni? so that it keeps reminding me to
> finish :)

But then we can't NI you a second time to ping you about it :)

Comment 23

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8613128 [details] [review]
[gaia] mikehenrty:bug-1166075-permission-shb > mozilla-b2g:master

Alberto, can you take a look here?

Comment 24

[Alberto Pastor [:albertopq]]

Comment on attachment 8613128 [details] [review]
[gaia] mikehenrty:bug-1166075-permission-shb > mozilla-b2g:master

LGTM. Thanks!

Comment 25

[Michael Henretty [:mikehenrty][:mhenretty]]

master: https://github.com/mozilla-b2g/gaia/commit/6caf6e5bdebc078556c87d254d144d38630f1fdf

Comment 26

[Queena.Wen(Leave from Mozilla)]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aries KK 2.5 by the STR in comment 0.

Actual results: The overlay covers the whole screen.
See attachment: verified_Aries_KK_v2.5.3gp
Reproduce rate: 0/10

Device: Flame KK 2.5 (Pass)
Build ID               20151007150205
Gaia Revision          b99837aa2294348317bcae68acabe71d9a83d774
Gaia Date              2015-10-07 13:04:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1e1fa696e2b626ead6817b7c5bd871fec5d5ab5a
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151007.183338
Firmware Date          Wed Oct  7 18:33:51 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20151008002716
Gaia Revision          b99837aa2294348317bcae68acabe71d9a83d774
Gaia Date              2015-10-07 13:04:16
Gecko Revision         https://hg.mozilla.org/mozilla-
central/rev/c6ede6f30f3dc886543bb1c76fd7c8b5a151786b
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151007.234555
Firmware Date          Wed Oct  7 23:46:03 UTC 2015
Bootloader             s1

Comment 27

[Queena.Wen(Leave from Mozilla)]

Attachment: verified_Aries_KK_v2.5.3gp