Bug 97631 - [EFL] No way to exit video fullscreen mode once entered
Summary: [EFL] No way to exit video fullscreen mode once entered
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-25 22:30 PDT by Chris Dumez
Modified: 2012-09-27 03:23 PDT (History)
12 users (show)

See Also:


Attachments
Patch (10.43 KB, patch)
2012-09-27 00:15 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-09-25 22:30:24 PDT
When switching a video to fullscreen in Minibrowser, the following controls are disappearing:
- Previous / Next buttons
- Fullscreen toggle
- Volume / Mute button

This means that we currently have to way to exit fullscreen mode once in fullscreen state and there is no way to change to volume in fullscreen mode.
Comment 1 Chris Dumez 2012-09-26 05:27:26 PDT
Gyuyoung, I see that you added the MediaControlFullScreen stylesheet for EFL port. Could you please explain why you're explicitly hiding those controls in full screen mode?

It seems useful (to me) to have volume control in full screen. The bigger issue is that we have currently no way of exiting full screen mode :( This is because there the fullscreen toggling button is hidden and we don't listen to keyboard input (e.g. ESC to exit).

A third issue that I'm still trying to debug:
1. Put a video in fullscreen mode (media controls on the right side are hidden)
2. Type the URL of another page which contains a video element in the URL bar
-> The video element in the new page has its right side controls hidden, even though in the not in fullscreen. This causes flakiness in our media tests as you can imagine.
Comment 2 Gyuyoung Kim 2012-09-26 17:34:44 PDT
(In reply to comment #1)
> Gyuyoung, I see that you added the MediaControlFullScreen stylesheet for EFL port. Could you please explain why you're explicitly hiding those controls in full screen mode?
> 
> It seems useful (to me) to have volume control in full screen. The bigger issue is that we have currently no way of exiting full screen mode :( This is because there the fullscreen toggling button is hidden and we don't listen to keyboard input (e.g. ESC to exit).
> 
> A third issue that I'm still trying to debug:
> 1. Put a video in fullscreen mode (media controls on the right side are hidden)
> 2. Type the URL of another page which contains a video element in the URL bar
> -> The video element in the new page has its right side controls hidden, even though in the not in fullscreen. This causes flakiness in our media tests as you can imagine.

I added fullscreen stylesheet by default. Yes, some key buttons are missing. I also think we need to add the exit button and toggle button. Keep going on please.
Comment 3 Chris Dumez 2012-09-27 00:15:18 PDT
Created attachment 165943 [details]
Patch
Comment 4 Gyuyoung Kim 2012-09-27 00:44:01 PDT
Comment on attachment 165943 [details]
Patch

LGTM.
Comment 5 Gyuyoung Kim 2012-09-27 01:40:09 PDT
Before landing this patch, I test some html 5 fullsceen demo site, for example,
- http://html5-demos.appspot.com/static/fullscreen.html
- http://gyuyoung.blogspot.kr/2011/07/html-5-video-demo.html

fullscreen button is not shown. In addition, fullscreen exit button is not shown in fullscreen mode as well. Could you check this ?
Comment 6 Gyuyoung Kim 2012-09-27 02:11:48 PDT
Comment on attachment 165943 [details]
Patch

I'd like to check my question first. Clear r+.
Comment 7 Kenneth Rohde Christiansen 2012-09-27 02:33:54 PDT
Comment on attachment 165943 [details]
Patch

Should ESC key work as well?
Comment 8 Chris Dumez 2012-09-27 02:37:02 PDT
(In reply to comment #7)
> (From update of attachment 165943 [details])
> Should ESC key work as well?

Yes, I think it is worth it for Desktop. It is planned but will be addressed in a separate patch.
Comment 9 Chris Dumez 2012-09-27 02:39:00 PDT
(In reply to comment #5)
> Before landing this patch, I test some html 5 fullsceen demo site, for example,
> - http://html5-demos.appspot.com/static/fullscreen.html
> - http://gyuyoung.blogspot.kr/2011/07/html-5-video-demo.html
> 
> fullscreen button is not shown. In addition, fullscreen exit button is not shown in fullscreen mode as well. Could you check this ?

Gyuyoung, it is probably a cache problem and a clean build should fix it.
Alternatively, I do the following before building:
touch Source/WebKit/efl/DefaultTheme/default.edc

This forces the edje files to be parsed again.

BTW, this is annoying and the edje file modification detection should be smarter.
Comment 10 Gyuyoung Kim 2012-09-27 02:42:23 PDT
If there is no problem, I am fine. Patch looks good itself.
Comment 11 WebKit Review Bot 2012-09-27 02:43:49 PDT
Comment on attachment 165943 [details]
Patch

Clearing flags on attachment: 165943

Committed r129745: <http://trac.webkit.org/changeset/129745>
Comment 12 WebKit Review Bot 2012-09-27 02:43:53 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Chris Dumez 2012-09-27 03:23:20 PDT
(In reply to comment #9)
> (In reply to comment #5)
> > Before landing this patch, I test some html 5 fullsceen demo site, for example,
> > - http://html5-demos.appspot.com/static/fullscreen.html
> > - http://gyuyoung.blogspot.kr/2011/07/html-5-video-demo.html
> > 
> > fullscreen button is not shown. In addition, fullscreen exit button is not shown in fullscreen mode as well. Could you check this ?
> 
> Gyuyoung, it is probably a cache problem and a clean build should fix it.
> Alternatively, I do the following before building:
> touch Source/WebKit/efl/DefaultTheme/default.edc
> 
> This forces the edje files to be parsed again.
> 
> BTW, this is annoying and the edje file modification detection should be smarter.

I'm fixing the CMake issue in Bug 97769.