871407 - Issues with direct link content download and playing.

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title : [BROWSER] Video files streaming from web sites in full screen mode causes a WEB Browser bottom bar UI problems
2. Precondition : 
3. Tester's Action : 
                1) Go to soem site which contails downloadable video content
                2) Click on the link, which normally is used to download it
                3) Watch the streamed video, switch to full screen, rotate to horizontal mode, turn off full screen, next turn on full screen again and rotate to vertical, next torn off full screen
4. Detailed Symptom (ENG.) : When leaving full screen in horizontal and vertical mode there is UI error in bottom Web Browser bar 
5. Expected : No UI problems
6.Reproducibility: Y
1)Frequency Rate : 100%
7.Comparison Results : 
1)Model Comparing : 
- Build ID : 20130421070203
- Gaia : 5cbb19e4bb78a7ad879fbe4b9a841e1c35714f5c
- Gecko : 950b402b6188bb2f3ce3176e620ed5249719d720

Comment 1

[Leo]

After we exit fullscreen its observed that the height of the "frame" element is increased by 20px. It happens only if we do a orientation change. This causes the toolbar to go beyond the screen boundary. but if we again force a resize (like keyboard show) original height value is restored.

Comment 2

[Ben Francis [:benfrancis]]

I can actually reproduce this using the UI test app on master too (though it's a lot harder to notice).

1. Open UI Test app
2. select the full screen controls page
3. Start to play video
4. Make video full screen
5. Switch to landscape mode
6. Exit full screen
7. Scroll all the way to the bottom of the page
8. Observe the amount of space between the bottom of the video and the bottom of the screen
9. Switch to portrait mode
10. Switch back to landscape mode
11. Scroll all the way to the bottom of the page again

Expected:
Same distance between bottom of video and bottom of screen

Actual:
Greater distance.

It looks like what is happening is that when the user exits full screen mode, the mozapp iframe which contains the app has a larger vertical dimension than it should have, possibly not taking into account the height of the status bar. If you switch between portrait and lanscape mode, the size is reset.

Moving to system app for further investigation in case the bug is in the CSS of the window manager, although it's possible this could be a Gecko layout bug.

Comment 3

[Leo]

Attachment: repro steps

Hi Alive, 
Would you mind if I ask for your quick look at this and pinpoint what/where could cause this?
Thanks!

Comment 4

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

It's strange that the inline frame size isn't maintained when orientation changed.
I'll take a look.

Comment 5

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

Uh, I couldn't reproduce either comment 0 or comment 2.

Comment 6

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

Attachment: Uitest->Fullscreen controls->Click video fullscreen->rotate

Comment 7

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

Comment on attachment 749626 [details]
Uitest->Fullscreen controls->Click video fullscreen->rotate

What step I miss?

Comment 8

[Leo]

Hi,

Please follow the below steps.

UITest->Fullscreen controls-> Play video -> make it full screen -> rotate to landscape -> rotate to portrait-> exit full screen-> scroll down

after that you can see that less space compared to original is visible.

Comment 9

[Leo]

Attachment: comparison

Comment 10

[Leo]

It is observed that the issue happens only when we do orientation change in full screen. So the issue may be due to,

* orientation change event will cause the application to resize.
* when the height is calculated in window manager, as its in fullscreen, statusbar height it will get as zero.
* exiting fullscreen wont cause a app resize in window manager as its not handled currently.

please confirm.

Also is there any particular reason why we are not handling fullscreen events in window manager?

Comment 11

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

Yes, we should.
So in the resize event array we could add one more event:

  var appResizeEvents = ['resize', 'status-active', 'status-inactive',
                         'keyboardchange', 'keyboardhide',
                         'attentionscreenhide', 'fullscreenchange'];
  appResizeEvents.forEach(function eventIterator(event) {
    window.addEventListener(event, function on(evt) {
      var keyboardHeight = KeyboardManager.getHeight();
      if (event == 'keyboardchange') {
        // Cancel fullscreen if keyboard pops
        if (document.mozFullScreen)
          document.mozCancelFullScreen();

        setAppSize(displayedApp, true, keyboardHeight, true);
      } else if (displayedApp) {
        setAppSize(displayedApp, true, keyboardHeight, false);
      }
    });
  });

Comment 12

[Leo]

Attachment: Pull request url

Comment 13

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

Comment on attachment 750335 [details]
Pull request url

r=alive, thanks.

Comment 14

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

merged.
https://github.com/mozilla-b2g/gaia/commit/2bfe0807ffa800ee46b77456b530e4a2c747ad70

Comment 15

[Leo]

(In reply to Alive Kuo [:alive] from comment #14)
> merged.
> https://github.com/mozilla-b2g/gaia/commit/
> 2bfe0807ffa800ee46b77456b530e4a2c747ad70

uplift to v1-train also?

Comment 16

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

This is not a leo+ bug, you need to ask gaia-approval for that.

Comment 17

[Leo]

(In reply to Alive Kuo [:alive] from comment #11)

I was reported, that adding 'fullscreenchange' doesn't solve the issue.
How about 'mozfullscreenchange' and 'webkitfullscreenchange'? We don't need to add those?

Thanks
Hanjkim

Comment 18

[Leo]

(In reply to Leo from comment #17)
From my testing now, the event received is 'mozfullscreenchange' in this case.

Alive, should we replace the 'fullscreenchange' with 'mozfullscreenchange'? Or simply just add 'mozfullscreenchange' in addition to the 'fullscreenchange'? 

Let me know, I'll create a new PR.
Hanjkim

Comment 19

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

(In reply to Leo from comment #18)
> (In reply to Leo from comment #17)
> From my testing now, the event received is 'mozfullscreenchange' in this
> case.
> 
> Alive, should we replace the 'fullscreenchange' with 'mozfullscreenchange'?
> Or simply just add 'mozfullscreenchange' in addition to the
> 'fullscreenchange'? 
> 
> Let me know, I'll create a new PR.
> Hanjkim

Should be mozfullscreenchange, please file another bug on it.