842898 - Let video app get rid of mozRequestFullScreen

Status: RESOLVED FIXED

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

Description

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

+++ This bug was initially created as a clone of Bug #833713 +++

1. Title : Music playback menu controls(play/pause icons..),shown at the bottom are cut(not full/clearly displayed) 
2. Precondition : Have music and video files on device
3. Tester's Action : Play a music->pause->press home screen->launch a video app->play video->pause video->long press on home button->select and open music app->the playback icons are cut 
4. Detailed Symptom (ENG.) : 
			1. Play and pause a music file ,go to home screen and launch a video play and pause
			2. Go to home long key, play music icons are PARTIALLY CUT
5. Gaia Revision # :   "7b8dea7e7ec377a2143151bbe5e9998d87f7b36d"
6. Expected :  Music apps PLAYBACK icons should be fully and clearly visible WITH OUT ANY CUTS.
7.Reproducibility: Yes
            1)Frequency Rate : 100%
8.Comparison Results : 
           1)Model Comparing : 
9. Attached files: 
           1)Log : 
           2)Test Contents : 
           3)Video file :

Comment 1

[Jason Smith [:jsmith]]

What's the qawanted request here? I'm assuming this was specified by the clone of the bug. Is there a specific need on this bug?

Comment 2

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/8467

patch v1:
* Get rid of mozRequestFullScreen
* Don't back to video list when video app falls into bg.

Comment 3

[David Flanagan [:djf]]

Alive: have you tested to see if this patch resolves both of the bugs that this bug blocks?

Comment 4

[David Flanagan [:djf]]

Comment on attachment 721142 [details]
https://github.com/mozilla-b2g/gaia/pull/8467

Thanks for working on this, Alive. I'll be happy to not use mozRequestFullscreen() anymore!

There are a few more changes you need to make, I think, so r- for now, but it should be easy to fix.  See my comments on github.

Comment 5

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

(In reply to David Flanagan [:djf] from comment #3)
> Alive: have you tested to see if this patch resolves both of the bugs that
> this bug blocks?

Yes. Both work fine. They should be marked as dup though.

Comment 6

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

Comment on attachment 721142 [details]
https://github.com/mozilla-b2g/gaia/pull/8467

David, thanks for the review. I change all you address in the comments and test again.

BTW, I find delete video doesn't work for me even using gaia master. 
The deleted video still reveals in the list even when I restart video app.
I assume this is because my gecko is too old? Or is this a known issue?

Comment 7

[David Flanagan [:djf]]

The file deletion code seems to be broken because the confirm() method stopped working.  I see this in the logcat when I try to delete with the delete button or long press


E/GeckoConsole(12728): [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWindowWatcher.openWindow]" {file: "jar:file:///system/b2g/omni.ja!/components/nsPrompter.js" line: 379}]

Comment 8

[David Flanagan [:djf]]

If I update to the latest unagi nightly build, then the confirm() method works again, and so does video deletion.

Comment 9

[David Flanagan [:djf]]

Comment on attachment 721142 [details]
https://github.com/mozilla-b2g/gaia/pull/8467

Nice work. The code looks good, and everything seems to work right in testing.

Sorry for the long delay on the review. Because of the timezone difference, I'll go ahead and land it.

Comment 10

[David Flanagan [:djf]]

Merged into Gaia master: https://github.com/mozilla-b2g/gaia/commit/ecd1ba101051b26bcebe2f0812bd1e7c6b9fec2f

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit ecd1ba101051b26bcebe2f0812bd1e7c6b9fec2f as:
v1-train: 0bcae2aec9b0f980327a128b7e4c364de0891ead

Comment 12

[:CSchmoeckel]

Added Music Suite Test Case #8613 [Music] The Music app's play controls display correctly after switching back from the Video App