113009 2013-03-21 23:30:12 -0700 [EFL][WK2] View is not focused when fullscreen mode toggled 2013-03-22 09:58:01 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 naginenis naginenis gyuyoung.kim lucas.de.marchi webkit.review.bot oldest_to_newest 861142 0 naginenis 2013-03-21 23:30:12 -0700 View is not focused when fullscreen mode enabled/disabled by FullScreen API. 861152 1 194455 naginenis 2013-03-21 23:42:32 -0700 Created attachment 194455 patch 861230 2 194455 laszlo.gombos 2013-03-22 01:46:48 -0700 Comment on attachment 194455 patch lgtm. 861236 3 194455 naginenis 2013-03-22 01:56:24 -0700 Comment on attachment 194455 patch Thanks Laszlo. 861260 4 194455 webkit.review.bot 2013-03-22 02:26:06 -0700 Comment on attachment 194455 patch Clearing flags on attachment: 194455 Committed r146581: <http://trac.webkit.org/changeset/146581> 861261 5 webkit.review.bot 2013-03-22 02:26:09 -0700 All reviewed patches have been landed. Closing bug. 861326 6 194455 cdumez 2013-03-22 04:04:47 -0700 Comment on attachment 194455 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. 861363 7 194455 naginenis 2013-03-22 05:18:26 -0700 Comment on attachment 194455 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. 861373 8 194455 cdumez 2013-03-22 05:35:09 -0700 Comment on attachment 194455 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. 861590 9 194455 naginenis 2013-03-22 09:58:01 -0700 Comment on attachment 194455 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 :) 194455 2013-03-21 23:42:32 -0700 2013-03-22 09:58:01 -0700 patch 113009.patch text/plain 1499 naginenis ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBiNWJm OTA1Li5kMzhhYTdjIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTMtMDMtMjEgIFN1ZGFyc2FuYSBOYWdpbmVuaSAg PHN1ZGFyc2FuYS5uYWdpbmVuaUBpbnRlbC5jb20+CisKKyAgICAgICAgW0VGTF1bV0syXSBWaWV3 IGlzIG5vdCBmb2N1c2VkIHdoZW4gZnVsbHNjcmVlbiBtb2RlIHRvZ2dsZWQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMzAwOQorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFNldCBmb2N1cyBvbiB2aWV3IHdo ZW4gZnVsbHNjcmVlbiBtb2RlIGVuYWJsZWQvZGlzYWJsZWQKKyAgICAgICAgYnkgRnVsbFNjcmVl biBBUEkuCisKKyAgICAgICAgKiBNaW5pQnJvd3Nlci9lZmwvbWFpbi5jOgorICAgICAgICAob25f ZnVsbHNjcmVlbl9hY2NlcHQpOgorICAgICAgICAob25fZnVsbHNjcmVlbl9kZW55KToKKwogMjAx My0wMy0yMSAgRGlyayBQcmFua2UgIDxkcHJhbmtlQGNocm9taXVtLm9yZz4KIAogICAgICAgICBu cnd0IGlzIG5vIGxvbmdlciBzYXZpbmcgcmV0cnkgcmVzdWx0cyBpbiB0aGUgcmV0cmllcy8gc3Vi ZGlyZWN0b3J5CmRpZmYgLS1naXQgYS9Ub29scy9NaW5pQnJvd3Nlci9lZmwvbWFpbi5jIGIvVG9v bHMvTWluaUJyb3dzZXIvZWZsL21haW4uYwppbmRleCA1ZjhjOTQ5Li41N2M4ZWZkIDEwMDY0NAot LS0gYS9Ub29scy9NaW5pQnJvd3Nlci9lZmwvbWFpbi5jCisrKyBiL1Rvb2xzL01pbmlCcm93c2Vy L2VmbC9tYWluLmMKQEAgLTEwMDUsNiArMTAwNSw3IEBAIG9uX2Z1bGxzY3JlZW5fYWNjZXB0KHZv aWQgKnVzZXJfZGF0YSwgRXZhc19PYmplY3QgKm9iaiwgdm9pZCAqZXZlbnRfaW5mbykKICAgICBQ ZXJtaXNzaW9uRGF0YSAqcGVybWlzc2lvbl9kYXRhID0gKFBlcm1pc3Npb25EYXRhICopdXNlcl9k YXRhOwogCiAgICAgZXZhc19vYmplY3RfZGVsKHBlcm1pc3Npb25fZGF0YS0+cGVybWlzc2lvbl9w b3B1cCk7CisgICAgZXZhc19vYmplY3RfZm9jdXNfc2V0KHBlcm1pc3Npb25fZGF0YS0+ZXdrX3Zp ZXcsIEVJTkFfVFJVRSk7CiAgICAgZnJlZShwZXJtaXNzaW9uX2RhdGEpOwogfQogCkBAIC0xMDE1 LDYgKzEwMTYsNyBAQCBvbl9mdWxsc2NyZWVuX2Rlbnkodm9pZCAqdXNlcl9kYXRhLCBFdmFzX09i amVjdCAqb2JqLCB2b2lkICpldmVudF9pbmZvKQogCiAgICAgZXdrX3ZpZXdfZnVsbHNjcmVlbl9l eGl0KHBlcm1pc3Npb25fZGF0YS0+ZXdrX3ZpZXcpOwogICAgIGV2YXNfb2JqZWN0X2RlbChwZXJt aXNzaW9uX2RhdGEtPnBlcm1pc3Npb25fcG9wdXApOworICAgIGV2YXNfb2JqZWN0X2ZvY3VzX3Nl dChwZXJtaXNzaW9uX2RhdGEtPmV3a192aWV3LCBFSU5BX1RSVUUpOwogICAgIGZyZWUocGVybWlz c2lvbl9kYXRhKTsKIH0KIAo=