Bug 113009 – [EFL][WK2] View is not focused when fullscreen mode toggled

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Sudarsana Nagineni (babu)]

View is not focused when fullscreen mode enabled/disabled by FullScreen API.

[Sudarsana Nagineni (babu)]

Created attachment 194455 [details]
patch

[Laszlo Gombos]

Comment on attachment 194455 [details]
patch

lgtm.

[Sudarsana Nagineni (babu)]

Comment on attachment 194455 [details]
patch

Thanks Laszlo.

[WebKit Review Bot]

Comment on attachment 194455 [details]
patch

Clearing flags on attachment: 194455

Committed r146581: <http://trac.webkit.org/changeset/146581>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Comment on attachment 194455 [details]
patch

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

> Tools/MiniBrowser/efl/main.c:1008
> +    evas_object_focus_set(permission_data->ewk_view, EINA_TRUE);

You're supposed to call view_focus_set() not evas_object_focus_set(). Otherwise, an elementary object may keep the focus.

> Tools/MiniBrowser/efl/main.c:1019
> +    evas_object_focus_set(permission_data->ewk_view, EINA_TRUE);

Ditto.

[Sudarsana Nagineni (babu)]

Comment on attachment 194455 [details]
patch

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

>> Tools/MiniBrowser/efl/main.c:1008
>> +    evas_object_focus_set(permission_data->ewk_view, EINA_TRUE);
> 
> You're supposed to call view_focus_set() not evas_object_focus_set(). Otherwise, an elementary object may keep the focus.

Initially I also thought about calling view_focus_set(), but I don't see any issues if we don't call that here. May be it is needed only in case of stealing focus away from the url_bar.

[Chris Dumez]

Comment on attachment 194455 [details]
patch

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

>>> Tools/MiniBrowser/efl/main.c:1008
>>> +    evas_object_focus_set(permission_data->ewk_view, EINA_TRUE);
>> 
>> You're supposed to call view_focus_set() not evas_object_focus_set(). Otherwise, an elementary object may keep the focus.
> 
> Initially I also thought about calling view_focus_set(), but I don't see any issues if we don't call that here. May be it is needed only in case of stealing focus away from the url_bar.

It is useful when stealing away focus from any elm widget. Depending on who owns the focus when you call evas_object_focus_set(), your change may not work. We introduced view_focus_set() exactly for this reason and it should be used. There is no drawback to using view_focus_set() but using evas_object_focus_set() may not work as expected in some cases.

[Sudarsana Nagineni (babu)]

Comment on attachment 194455 [details]
patch

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

>>>> Tools/MiniBrowser/efl/main.c:1008
>>>> +    evas_object_focus_set(permission_data->ewk_view, EINA_TRUE);
>>> 
>>> You're supposed to call view_focus_set() not evas_object_focus_set(). Otherwise, an elementary object may keep the focus.
>> 
>> Initially I also thought about calling view_focus_set(), but I don't see any issues if we don't call that here. May be it is needed only in case of stealing focus away from the url_bar.
> 
> It is useful when stealing away focus from any elm widget. Depending on who owns the focus when you call evas_object_focus_set(), your change may not work. We introduced view_focus_set() exactly for this reason and it should be used. There is no drawback to using view_focus_set() but using evas_object_focus_set() may not work as expected in some cases.

I agree with you, evas_object_focus_set() doesn't work in some cases. But, in this particular case I don't see an issue by not calling view_focus_set(). Anyway, I don't mind uploading a new patch, if really needed :)