1037255 - [User Story] Hide soft home key during full screen video playback

Status: VERIFIED FIXED

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

Description

[Peter Dolanjski [:pdol]]

As a user, I want the soft home key to be hidden during full screen video playback so that it doesn't interfere with the video on the screen.

Acceptance Criteria:
1. After video starts playing full screen, the soft home key hides, per the UX spec.
2. The user is able to bring back the home button during full screen video playback, per the UX spec.
3. Interaction and visual design matches UX spec.

Comment 1

[Markus Nilsson]

Attachment: Github patch

Attached a patch with the fullscreen soft home part of the patch I previously added to Bug 1032768.

Francis, please review this instead of Bug 1032768.

Comment 2

[Francis Djabri [:djabber]]

Re-assigning to Rob since he's back from his long weekend. Rob, let me know if you're still having trouble getting your phone to work.

Comment 3

[Rob MacDonald [:robmac]]

Hi Markus... I was able to review the patch but it's difficult to comment as it has not been implemented with the playback controls. The only potential irregularity I noticed using the UI test tool was that, after the soft home button was hidden, it would take two taps to re-activate the button. This isn't in the spec but perhaps there's something I'm missing. - Rob

Comment 4

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

Comment on attachment 8467744 [details] [review]
Github patch

Etienne, let's start the review process for this part while bug 1054126 gets under way.

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 8467744 [details] [review]
Github patch

(Repeating some of the discussions from last week to make sure everybody is up to date.)

We want to make it easy for a video app to synchronize its own video controls with the system software home button.
For apps with the "fullscreen_layout" property in their manifest, mozRequestFullScreen() will hint at the system app that the home button should be hidden, mozCancelFullScreen() will hint that it should be displayed.
We also don't want an app to lock the user by hiding the home button, so while in "mozRequestFullScreen() mode" taping anywhere will bring the software home button on screen.

For our use case:
* when the video starts, the app hides its controls and call mozRequestFullScreen() to make the system hide the home button
* when the user taps the screen, the system and the app both get a 'click' event, the system brings back the software home and the app should bring back the video controls and call mozCancelFullScreen() to make sure the software home won't go away after a timeout.

For this to work we need to make sure of the followings:
* mozRequestFullScreen() hides the software home right away
* mozCancelFullScreen() shows the software home right away
* while in fullscreen, the first 'click' toggles the software home right away
* (outside the scope of this bug) the app should be able to call mozRequestFullScreen()/mozCancelFullScreen() without causing an ugly flash on screen

We should of course do a nice transition when the software home comes one/off screen, but those code paths should not include any timeouts.

It's perfectly okay to keep the timeout where the software home buttons hides itself 2 seconds after a click if the app stayed in fullscreen. But it's important to reset `tapShowsHome` to true when the fullscreen is canceled.

I also believe that removing the DOM duplication with #software-buttons and #fullscreen-software-buttons could help us make the patch simpler and the transition smoother.
Typically while in fullscreen, a user tap will make the system app show the software home button, but at the same time the app will probably call mozCancelFullScreen() and thus also making us show the software home button. If we have only 1 div in the dom where we add the .visible class twice that's no problem, the transition will happily continue.

(Feel free to ping me on IRC if there's anything unclear about this way too long comment :))

Comment 6

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

Looking at this patch, it does not include the changes we need for the Video app. It only allows showing and hiding of the software home button using the Fullscreen API. So I filed bug 1055198 to track the video app work.

Comment 7

[Markus Nilsson]

Some thoughts:

The idea that we should reuse the #software-buttons element for this feature is a good one, although we'll need to keep the #fullscreen-software-home-button for apps that doesn't use the manifest setting.

The 'tapShowsHome' code should no longer be needed. The thought behind it was that the soft home bar might cover a button in the app, so if we didn't show the soft home every time, the user would still be able to get to that button. But since this is now opt-in, it's reasonable to assume that no app that use it will put a button in that area.

We probably want to adjust the 2 second hide time-out so it aligns with the hiding of the video controls.

We might want to allow swipes without showing the home button. I'm thinking of a album/gallery app where you want to show pictures in fullscreen and have the ability to swipe between them without showing the soft home.

Comment 8

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #7)
> Some thoughts:
> 
> The idea that we should reuse the #software-buttons element for this feature
> is a good one, although we'll need to keep the
> #fullscreen-software-home-button for apps that doesn't use the manifest
> setting.

Can you explain why?
Since it's easy to mix things up between fullscreen in manifest, fullscreen_layout in manifest and mozRequestFullScreen() I want to make sure I'm following you correctly.
 
> We probably want to adjust the 2 second hide time-out so it aligns with the
> hiding of the video controls.

Yes, but the ideal video app should call mozRequestFullScreen() when hiding the controls to make sure the software home button hides too.

> 
> We might want to allow swipes without showing the home button. I'm thinking
> of a album/gallery app where you want to show pictures in fullscreen and
> have the ability to swipe between them without showing the soft home.

Makes sense, we could listen to touchstart/touchend (those are also dispatched to the app and the system) instead of click to ignore the cases where the finger moved pass a threshold.

Comment 9

[Markus Nilsson]

(In reply to Etienne Segonzac (:etienne) from comment #8)
> (In reply to Markus Nilsson from comment #7)
> > Some thoughts:
> > 
> > The idea that we should reuse the #software-buttons element for this feature
> > is a good one, although we'll need to keep the
> > #fullscreen-software-home-button for apps that doesn't use the manifest
> > setting.
> 
> Can you explain why?
> Since it's easy to mix things up between fullscreen in manifest,
> fullscreen_layout in manifest and mozRequestFullScreen() I want to make sure
> I'm following you correctly.
>  
Sorry, I probably caused confusion by talking about #fullscreen-software-home-button and not #fullscreen-software-home-buttons. What I meant was just that apps that doesn't have fullscreen_layout in their manifest will still be able call mozRequestFullScreen() and will then get a soft home button without the black soft home bar, i.e. the #fullscreen-software-home-button we currently have without this patch.

That said, after thinking some more about using the same element, I'm not so sure that it's a good idea.
If you remember, at first when I did Bug 1032768 I didn't just transition the position of the soft home bar, I transitioned the size of the content as well. That caused bad performance and the solution was to just resize the content once when the soft home bar had transitioned into place and that looked okay when we're in a list in the settings app.

In your example:
> or our use case:
> * when the video starts, the app hides its controls and call mozRequestFullScreen() to make the system
> hide the home button
> * when the user taps the screen, the system and the app both get a 'click' event, the system brings back > the software home and the app should bring back the video controls and call mozCancelFullScreen() to make > sure the software home won't go away after a timeout.

So the user taps the screen and mozCancelFullScreen() is called. If the soft home bar is transitioned into place, what size is the content at the start of that transition? If it's the full screen, then we'll resize once the soft home bar is in place, which probably won't look that good. If it's full screen minus the size of the soft home bar then the homescreen will probably be visible in the gap.
The same thing will probably occur if you get a phone call while you're watching a video, how do we bring back the soft home in a good way without resize problems?

Another thought, could a malicious app call mozRequestFullScreen() every time the user taps the screen to deny access to the soft home button?

Comment 10

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #9)
> Sorry, I probably caused confusion by talking about
> #fullscreen-software-home-button and not #fullscreen-software-home-buttons.
> What I meant was just that apps that doesn't have fullscreen_layout in their
> manifest will still be able call mozRequestFullScreen() and will then get a
> soft home button without the black soft home bar, i.e. the
> #fullscreen-software-home-button we currently have without this patch.
> 
> That said, after thinking some more about using the same element, I'm not so
> sure that it's a good idea.
> If you remember, at first when I did Bug 1032768 I didn't just transition
> the position of the soft home bar, I transitioned the size of the content as
> well. That caused bad performance and the solution was to just resize the
> content once when the soft home bar had transitioned into place and that
> looked okay when we're in a list in the settings app.

Good point. And having 2 dom nodes is not a big deal if it's actually helping.

> 
> In your example:
> > or our use case:
> > * when the video starts, the app hides its controls and call mozRequestFullScreen() to make the system
> > hide the home button
> > * when the user taps the screen, the system and the app both get a 'click' event, the system brings back > the software home and the app should bring back the video controls and call mozCancelFullScreen() to make > sure the software home won't go away after a timeout.
> 
> So the user taps the screen and mozCancelFullScreen() is called. If the soft
> home bar is transitioned into place, what size is the content at the start
> of that transition? If it's the full screen, then we'll resize once the soft
> home bar is in place, which probably won't look that good. If it's full
> screen minus the size of the soft home bar then the homescreen will probably
> be visible in the gap.
> The same thing will probably occur if you get a phone call while you're
> watching a video, how do we bring back the soft home in a good way without
> resize problems?

An app using the requestFullScreen() API to give us hints about the software home button should definitely also opt-in to the fullscreen_layout :)

For apps that don't, the transition when requesting/canceling fullscreen will never be ideal.
I think canceling the fullscreen should bring you back instantly to a screen where the software home button is already there.
And I agree that this is a good argument to keep the 2 separate dom nodes.

> 
> Another thought, could a malicious app call mozRequestFullScreen() every
> time the user taps the screen to deny access to the soft home button?

If the app is already in "requested fullscreen", doing mozRequestFullScreen() shouldn't do anything (so the button will appear as expected).

We should definitely investigate what happens if an app calls cancel/request in the same tick but hopefully gecko is already hardened against that.

Comment 11

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

Markus, has there been any movement on this one? Is it clear how to go forward, or are you blocked?

Comment 12

[Markus Nilsson]

My work is progressing but it depends on Bug 1054126 and the "fullscreen_layout" and "Software home button status" mail discussions we're having.
If nothing unexpected pops up from those discussions I expect to have a new PR ready tomorrow or early Wednesday.

Comment 13

[Markus Nilsson]

Comment on attachment 8467744 [details] [review]
Github patch

I've updated the PR.
Please note that it has a dependency to Bug 1054126.

Comment 14

[Etienne Segonzac (:etienne)]

Comment on attachment 8467744 [details] [review]
Github patch

It's *really* cool to see this coming together (tried it on top of George's patch).

Made some comments on github, the main issue we need to talk through is the transition.

Specifically going into "requested fullscreen mode" and going out of "requested fullscreen mode" when the home button is hidden.

I frequently see the software home flash on screen when requesting fullscreen (one frame with the fullscreen element on top of the #software-home-button, the next frame with the #fullscreen-layout-software-home-button finally rendered).

And when canceling fullscreen, if the #fullscreen-layout-software-home-button is hidden, the #software-home-button just flash since it goes back on top of the frame.

What we want in fullscreenlayout mode is:
- when requesting fullsreen nothing should move or flash
- when canceling it, if the software home was hidden, it should fade back on screen

Since all we get in 1 mozfullscreenchange event the only way to do this, I think, is to have the fullscreen-layout-software-home-button element always displayed, opacity:1 by default. The _fullscreenTapFunction should just toggle a ".hidden" css class transitioning the opacity to 0. When we get a mozfullscreenchange we should remove the .hidden class.

We should never see #software-buttons when the current app is a fullscreen_layout app, it's not required but we could even hide it.

Let me know if there's anything unclear, let's make this smooth :)

Comment 15

[Markus Nilsson]

I've updated the PR, please try it out.
I feel a bit uncomfortable with setting z-index to 2147483647 when we're not in fullscreen, but so far I haven't found any issues with it.

Comment 16

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR

We've been coworking with Markus on the finishing touches (mainly unit tests) and here is the result squashed into one commit.

We're waiting for try :)

Comment 17

[Etienne Segonzac (:etienne)]

There were some build system intermittents but there's no way we're causing them *and* it's green now.
When the tree opens this is ready to land.

*victory dance*

https://tbpl.mozilla.org/?rev=5101b82118c8eb4ad3f24c0f4373a7e4c8cc77d0&tree=Gaia-Try

Comment 18

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

master: https://github.com/mozilla-b2g/gaia/commit/0f24c1b426004422907d465a4b7c9b18ac3d8ff8

Comment 19

[KTucker [:KTucker][Inactive 3/4/2016]]

I don't think there is a way for me to verify this issue since there is other work that needs to be done in the video app to get the software home button to hide when watching a video fullscreen. This is being done in bug 1055198

Comment 20

[Jason Smith [:jsmith]]

Discussed offline with Mike - this feature is only intended to be supported on Tako for 2.1, not Flame. Therefore, there's no QA verification required here on Flame, so marking as verified.

Comment 21

[Shine(Leave from Mozilla)]

Dear Peter,
Could you please provide a repro video, thanks!

Comment 22

[Peter Dolanjski [:pdol]]

(In reply to Shine from comment #21)
> Dear Peter,
> Could you please provide a repro video, thanks!

This isn't a bug.  It was a new feature, so there wouldn't be a repro video.