Bug 139482 - [EFL] MiniBrowser steals web view's focus
Summary: [EFL] MiniBrowser steals web view's focus
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: Grzegorz Czajkowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-10 07:35 PST by Grzegorz Czajkowski
Modified: 2014-12-22 07:30 PST (History)
5 users (show)

See Also:


Attachments
test case (169 bytes, text/html)
2014-12-10 07:35 PST, Grzegorz Czajkowski
no flags Details
Patch (1.59 KB, patch)
2014-12-10 07:39 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff
Transfer focus to ewk_view when main window gets focused (5.34 KB, patch)
2014-12-12 06:19 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2014-12-10 07:35:11 PST
Created attachment 243018 [details]
test case

Although we set focus on ewk_view while MiniBrowser initialization Ecore steals it in a favor of main window


#0  0x0000000000407285 in on_ewk_view_focus_out ()
#1  0x00007ffff5096073 in _eo_evas_object_cb () at lib/evas/canvas/evas_callbacks.c:84
#2  0x00007fffe7af9f9a in _eo_base_event_callback_call () at lib/eo/eo_base_class.c:713
#3  0x00007fffe7af82a9 in eo_event_callback_call () at lib/eo/eo_base.eo.c:102
#4  0x00007ffff5096599 in evas_object_event_callback_call () at lib/evas/canvas/evas_callbacks.c:256
#5  0x00007ffff50a56ef in _evas_object_focus_set () at lib/evas/canvas/evas_focus.c:40
#6  0x00007ffff50c4d8d in evas_obj_focus_set () at ../src/lib/evas/canvas/evas_object.eo.c:206
#7  0x00007ffff50a564e in _evas_object_focus_set () at lib/evas/canvas/evas_focus.c:27
#8  0x00007ffff50c4d8d in evas_obj_focus_set () at ../src/lib/evas/canvas/evas_object.eo.c:206
#9  0x00007ffff50d19c1 in evas_object_focus_set () at ../src/lib/evas/canvas/evas_object.eo.c:716
#10 0x00007ffff5692ded in _elm_widget_on_focus () at elm_widget.c:5418
#11 0x00007ffff5685170 in elm_obj_widget_on_focus () at elm_widget.eo.c:343
#12 0x00007ffff56a2e81 in _elm_win_elm_widget_on_focus () at elm_win.c:1282
#13 0x00007ffff5685170 in elm_obj_widget_on_focus () at elm_widget.eo.c:343
#14 0x00007ffff5697063 in _parent_focus () at elm_widget.c:797
#15 0x00007ffff56971cb in _elm_widget_focus_steal () at elm_widget.c:2807
#16 0x00007ffff568655a in elm_obj_widget_focus_steal () at elm_widget.eo.c:427
#17 0x00007ffff56960ab in elm_widget_focus_steal () at elm_widget.eo.c:1296
#18 0x00007ffff56a0c25 in _elm_win_focus_in () at elm_win.c:980
#19 0x00007fff968432b3 in _ecore_evas_x_event_window_focus_in () at modules/ecore_evas/engines/x/ecore_evas_x.c:1397
#20 0x00007ffff625ea07 in _ecore_event_call () at lib/ecore/ecore_private.h:359
#21 0x00007ffff6265e55 in _ecore_main_loop_iterate_internal () at lib/ecore/ecore_main.c:1940
#22 0x00007ffff6266187 in ecore_main_loop_begin () at lib/ecore/ecore_main.c:983
#23 0x000000000040d57a in elm_main ()
#24 0x000000000040d5ed in main ()
Comment 1 Grzegorz Czajkowski 2014-12-10 07:39:45 PST
Created attachment 243019 [details]
Patch
Comment 2 Grzegorz Czajkowski 2014-12-10 21:30:31 PST
I'm not sure if this is correct fix, maybe we should set ewk_view focus after elm_win initialization? Thanks
Comment 3 Grzegorz Czajkowski 2014-12-12 06:19:15 PST
Created attachment 243196 [details]
Transfer focus to ewk_view when main window gets focused
Comment 4 Grzegorz Czajkowski 2014-12-15 02:23:18 PST
Hi Gyuyoung and Ryuan any comment on this?

Let me describe the difference between Patch #1 and #2.

First patch blocks programatically focus set on main window which Ecore set (in fact, stealing it from ewk_view) on application startup.
Drawback: since we manage focus manually, caret blinks on input fields when application goes background.

Second patch, transfers focus to ewk_view when main windows gets focused. As a result, when Ecore assigns focus to main window on application startup, in fact we give it to ewk_view. Additionally it fixes bugs (saving/restoring focus) when application goes background/foreground which fits common Browser behavior.
Comment 5 Gyuyoung Kim 2014-12-15 03:16:27 PST
Comment on attachment 243196 [details]
Transfer focus to ewk_view when main window gets focused

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

> Tools/ChangeLog:18
> +        Otherwise, cursor may sometimes appear simultaneously.

Similar bug seems to be filed before.

https://bugs.webkit.org/show_bug.cgi?id=136324#c0

Ryuan, any comment on this ?

> Tools/MiniBrowser/efl/main.c:1996
> +on_window_focus_in(void *user_data, Evas *e, Evas_Object *ewk_view, void *event_info)

I'm not sure whether EFL application should handle focus in/out. If we should, it is very hard to make EFL application based on ewebkit. I think we need to investigate if this work is normal behavior in EFL application.
Comment 6 Grzegorz Czajkowski 2014-12-19 05:01:52 PST
(In reply to comment #5)
> Comment on attachment 243196 [details]
> Transfer focus to ewk_view when main window gets focused
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243196&action=review
> 
> > Tools/ChangeLog:18
> > +        Otherwise, cursor may sometimes appear simultaneously.
> 
> 
> I think we need to investigate if this work is normal behavior in EFL application.

Not really, this is rather ewk_view (Evas_Object) specific bug as elm objects behave correctly. I did quick change by setting focus on url_bar (elm_entry) using elm_object_focus_set(window->url_bar, EINA_TRUE). In this case url_bar remains focus whereas ewk_view does not.

To prevent stealing focus from ewk_view while startup application I preprepared patch #1 (Show Obsolete if you are interested in reviewing it). However, after that cursor still blinks when application goes foreground but it might be more WebKit related bug, for example WKViewSetIsActive(false)? 

Having this bug in the trunk causes real difference between MiniBrowser and WebKitTestRunner, especially in editing commands (some of them do not even work for unfocused view).
Comment 7 Ryuan Choi 2014-12-22 01:23:45 PST
(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 243196 [details]
> > Transfer focus to ewk_view when main window gets focused
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=243196&action=review
> > 
> > > Tools/ChangeLog:18
> > > +        Otherwise, cursor may sometimes appear simultaneously.
> > 
> > 
> > I think we need to investigate if this work is normal behavior in EFL application.
> 
> Not really, this is rather ewk_view (Evas_Object) specific bug as elm
> objects behave correctly. I did quick change by setting focus on url_bar
> (elm_entry) using elm_object_focus_set(window->url_bar, EINA_TRUE). In this
> case url_bar remains focus whereas ewk_view does not.
> 
> To prevent stealing focus from ewk_view while startup application I
> preprepared patch #1 (Show Obsolete if you are interested in reviewing it).
> However, after that cursor still blinks when application goes foreground but
> it might be more WebKit related bug, for example WKViewSetIsActive(false)? 
> 
> Having this bug in the trunk causes real difference between MiniBrowser and
> WebKitTestRunner, especially in editing commands (some of them do not even
> work for unfocused view).

I preffered first option because it's simpler.
In fact, I think that both are not right way but I also know that there are not right solution for ewebkit on elm.
(One more hackish solution is using elm_box or similar container.)
If we can't provide right solution, I think that the simpler is the better.

FYI, there are two focus system in EFL. The first is for evas and the second is for elm.
The focus system of ELM controls not only its own focus but also evas's focus. ewebkit which is based on evas smart object can't know the focus of elm.
Comment 8 Grzegorz Czajkowski 2014-12-22 01:35:24 PST
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Comment on attachment 243196 [details]
> > > Transfer focus to ewk_view when main window gets focused
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=243196&action=review
> > > 
> > > > Tools/ChangeLog:18
> > > > +        Otherwise, cursor may sometimes appear simultaneously.
> > > 
> > > 
> > > I think we need to investigate if this work is normal behavior in EFL application.
> > 
> > Not really, this is rather ewk_view (Evas_Object) specific bug as elm
> > objects behave correctly. I did quick change by setting focus on url_bar
> > (elm_entry) using elm_object_focus_set(window->url_bar, EINA_TRUE). In this
> > case url_bar remains focus whereas ewk_view does not.
> > 
> > To prevent stealing focus from ewk_view while startup application I
> > preprepared patch #1 (Show Obsolete if you are interested in reviewing it).
> > However, after that cursor still blinks when application goes foreground but
> > it might be more WebKit related bug, for example WKViewSetIsActive(false)? 
> > 
> > Having this bug in the trunk causes real difference between MiniBrowser and
> > WebKitTestRunner, especially in editing commands (some of them do not even
> > work for unfocused view).
> 
> I preffered first option because it's simpler.
> In fact, I think that both are not right way but I also know that there are
> not right solution for ewebkit on elm.
> (One more hackish solution is using elm_box or similar container.)
> If we can't provide right solution, I think that the simpler is the better.
> 
> FYI, there are two focus system in EFL. The first is for evas and the second
> is for elm.
> The focus system of ELM controls not only its own focus but also evas's
> focus. ewebkit which is based on evas smart object can't know the focus of
> elm.

Thanks for your opinion. Let me mark patch #1 for review once again than.
Comment 9 Gyuyoung Kim 2014-12-22 06:47:32 PST
Comment on attachment 243019 [details]
Patch

LGTM
Comment 10 WebKit Commit Bot 2014-12-22 07:30:12 PST
Comment on attachment 243019 [details]
Patch

Clearing flags on attachment: 243019

Committed r177632: <http://trac.webkit.org/changeset/177632>
Comment 11 WebKit Commit Bot 2014-12-22 07:30:18 PST
All reviewed patches have been landed.  Closing bug.