Summary: | [EFL][WK2] View is not focused when fullscreen mode toggled | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sudarsana Nagineni (babu) <naginenis> | ||||
Component: | WebKit EFL | Assignee: | Sudarsana Nagineni (babu) <naginenis> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | gyuyoung.kim, lucas.de.marchi, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Sudarsana Nagineni (babu)
2013-03-21 23:30:12 PDT
Created attachment 194455 [details]
patch
Comment on attachment 194455 [details]
patch
lgtm.
Comment on attachment 194455 [details]
patch
Thanks Laszlo.
Comment on attachment 194455 [details] patch Clearing flags on attachment: 194455 Committed r146581: <http://trac.webkit.org/changeset/146581> All reviewed patches have been landed. Closing bug. 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. 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. 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. 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 :) |