Tests, in which focus is switched to subframe left FrameSelection on main frame unfocussed. It affects tests which follow broken one - "shouldBeginEditing" callbacks are not generated.
Created attachment 168374 [details] patch
Code itself looks good to me since it's done in reset stage between run of each test case. However, as focus/blur has strong impact on layout test, I would recommend you to run full layout test and check if regression happens. :-)
Comment on attachment 168374 [details] patch You say it fixes a problem but you don't unskip any test. So how do we know it is better now?
I've ran full layout tests without http ones, and this patch fixes ~15 tests. What's more important it fixes random failures in other tests(when child processes are greater than 1)
(In reply to comment #4) > I've ran full layout tests without http ones, and this patch fixes ~15 tests. What's more important it fixes random failures in other tests(when child processes are greater than 1) Then please unskip them in the same patch.
(In reply to comment #5) > (In reply to comment #4) > > I've ran full layout tests without http ones, and this patch fixes ~15 tests. What's more important it fixes random failures in other tests(when child processes are greater than 1) > > Then please unskip them in the same patch. The problem is that this patch doesn't fix specified tests, but general flow. Problematic test is passed, but has influence on next one, and this next one is randomic.
(In reply to comment #4) > I've ran full layout tests without http ones, and this patch fixes ~15 tests. What's more important it fixes random failures in other tests(when child processes are greater than 1) This would prove improvement on flaky test cases. :-)
Please unskip passed flaky test cases here. :-)
Comment on attachment 168374 [details] patch All this patch does it call: evas_object_focus_set(m_view, false); evas_object_focus_set(m_view, true); in a row, instead of simply calling: evas_object_focus_set(m_view, true); right? I don't get why this has any impact, why we need it, and why other ports don't. This fix does not look right to me.
(In reply to comment #9) > All this patch does it call: > evas_object_focus_set(m_view, false); > evas_object_focus_set(m_view, true); > These two lines of code is not so strange to me in practical project. Generally due to 'code guarding from another unknown bug' or 'library defect'. But, it couldn't be a solution. :-) @Mariusz, Would you please verify how focus works by using 'evas_focus_get'? You can refer to 'http://docs.enlightenment.org/auto/evas/Example_Evas_Events.html' for code details. :) Maybe, another component steal focus or efl has defect.
evas_object_focus_set makes no effect on already focused Evas_Object. Same on widgets in gtk. Problem exists also at least on gtk port. So making only focus at the beginning of test does nothing. Problem is inside webkit: There is focus on webview(Evas_Object), Frame, FocusController and FrameSelection If some test using subframe switch focus to it, subframe takes focus, and on test exit it is destroyed, so none frame is focused, but webview is. The problem is that after this operation in following test main frame has FrameSelection unfocussed so editing callbacks don't come. Patch makes unfocus on webview so all components are blurred, than on focus all of them takes focus: FocusController and FrameSelection.
Comment on attachment 168374 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=168374&action=review You should unskip the flaky tests that it fixes in the same patch otherwise we cannot tell that it changes anything. > Tools/WebKitTestRunner/TestController.cpp:518 > + m_mainWebView->blur(); We - at least - need a comment here to explain why the EFL port needs this.
I think gtk+ port reviewers should know this kind of behavior since you mentioned gtk+ port has same issue.
(In reply to comment #12) > (From update of attachment 168374 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168374&action=review > > You should unskip the flaky tests that it fixes in the same patch otherwise we cannot tell that it changes anything. In editing category there's a lot of skipped subcategories(deleting, inserting, selection, spelling, style). A lot have also missing expected results. Some of them have different cause of being skipped. To check this change you can run following 2 tests one after other: editing/undo/undo-iframe-location-change.html LayoutTests/editing/undo/undo-indent.html Without this patch you can see following lines in diff file: -EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of OL > DIV > BODY > HTML > #document to 9 of OL > DIV > BODY > HTML > #document -EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification I suggest make gardening in separate patch including this change. > > > Tools/WebKitTestRunner/TestController.cpp:518 > > + m_mainWebView->blur(); > > We - at least - need a comment here to explain why the EFL port needs this. In DRT following code at the beginning of each test solves this problem: ewk_frame_feed_focus_in(mainFrame());
Any update ?
Comment on attachment 168374 [details] patch Cleared review? from attachment 168374 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug or this bug again.
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.