Bug 136854 – [EFL][WK2] Minibrowser : Fix the 'Escape' button issue to exit fullscreen

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Rohit]

Escape button does not work when try to exit Minibrowser fullscreen mode.

[Rohit]

Created attachment 238176 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 238176 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238176&action=review

> Tools/MiniBrowser/efl/main.c:-565
> -            ewk_view_fullscreen_exit(ewk_view);

Doesn't this function work now ?

[Rohit]

(In reply to comment #2)
> (From update of attachment 238176 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238176&action=review
> 
> > Tools/MiniBrowser/efl/main.c:-565
> > -            ewk_view_fullscreen_exit(ewk_view);
> 
> Doesn't this function work now ?

This function doesn't work now. I think reason behind this is that the Webview does not know that it has gone full screen.
We use elm_win_fullscreen_set to toggle full screen and elm_window has fullscreen state information, and not the ewk_view. There is no specific API calls to enter full screen for ewk_view. So ewk_view_fullscreen_exit doesn't work as ewk_view doesn't have information about current fullscreen state.

[Gyuyoung Kim]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 238176 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=238176&action=review
> > 
> > > Tools/MiniBrowser/efl/main.c:-565
> > > -            ewk_view_fullscreen_exit(ewk_view);
> > 
> > Doesn't this function work now ?
> 
> This function doesn't work now. I think reason behind this is that the Webview does not know that it has gone full screen.
> We use elm_win_fullscreen_set to toggle full screen and elm_window has fullscreen state information, and not the ewk_view. There is no specific API calls to enter full screen for ewk_view. So ewk_view_fullscreen_exit doesn't work as ewk_view doesn't have information about current fullscreen state.

If so, I think we should remove the APIs, Probably new bug ?

[Gyuyoung Kim]

Comment on attachment 238176 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238176&action=review

Basically patch looks good. r=me. Please update ChangeLog before landing.

> Tools/ChangeLog:7
> +

It would be nicer if you mention what is problem, how to fix this problem.

[Rohit]

Created attachment 238468 [details]
Patch for landing

[Gyuyoung Kim]

Comment on attachment 238468 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=238468&action=review

> Tools/ChangeLog:6
> +        Reviewed by Gyuyoung Kim.

Add a new line.

[Ryuan Choi]

Comment on attachment 238468 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=238468&action=review

> Tools/ChangeLog:9
> +        ewk_view_fullscreen_exit(ewk_view) doesn't work as ewk_view does not know that it has gone fullscreen.
> +        We use elm_win_fullscreen_set to toggle fullscreen and elm_window has fullscreen state information, and not the ewk_view.
> +        There is no specific API calls to enter fullscreen for ewk_view. So either we need to add API for enter fullscreen or remove ewk_view_fullscreen_exit.

Sorry for late comment.

But, I think that this comment looks wrong.

This issue is the bug of MiniBrowser.
ewk_view_fullscreen_exit() is to notify the changes of state to ewk_view when user cancelled HTML5's fullscreen API.
For example, w3c fullscreen API can make specific element as full screen (ex, video element).
So, ewk_view should know the exit to restore the web contents from specific element to whole contents when user cancelled requests of w3c fullscreen.

I think that MiniBrowser should distinguish which events started current full screen status, w3c's fullscreen API or shortcut(F11)

[Rohit]

(In reply to comment #8)
> (From update of attachment 238468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238468&action=review
> 
> > Tools/ChangeLog:9
> > +        ewk_view_fullscreen_exit(ewk_view) doesn't work as ewk_view does not know that it has gone fullscreen.
> > +        We use elm_win_fullscreen_set to toggle fullscreen and elm_window has fullscreen state information, and not the ewk_view.
> > +        There is no specific API calls to enter fullscreen for ewk_view. So either we need to add API for enter fullscreen or remove ewk_view_fullscreen_exit.
> 
> Sorry for late comment.
> 
> But, I think that this comment looks wrong.
> 
> This issue is the bug of MiniBrowser.
> ewk_view_fullscreen_exit() is to notify the changes of state to ewk_view when user cancelled HTML5's fullscreen API.
> For example, w3c fullscreen API can make specific element as full screen (ex, video element).
> So, ewk_view should know the exit to restore the web contents from specific element to whole contents when user cancelled requests of w3c fullscreen.
> 
> I think that MiniBrowser should distinguish which events started current full screen status, w3c's fullscreen API or shortcut(F11)

Thanks for the insight. It makes sense. On observing behaviours for other broswers, I found that 'Esc'is used only for exiting fullscreen when a specific element is fullscreen and not in case of window fullscreen. In case when browser window is fullscreen, F11 is used to exit fullscreen. However, F11 can be used to exit fullscreen even when a window element has gone fullscreen as specific element makes window go fullscreen along with it. I will submit the modified patch based on this observation.

[Rohit]

Created attachment 238531 [details]
Patch

[Ryuan Choi]

Looks good to me.

[Gyuyoung Kim]

Comment on attachment 238531 [details]
Patch

LGTM too.

[WebKit Commit Bot]

Comment on attachment 238531 [details]
Patch

Clearing flags on attachment: 238531

Committed r173997: <http://trac.webkit.org/changeset/173997>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.