LayoutTestController needs implementation of setTabKeyCyclesThroughElements to unskip fast/events/keypress-insert-tab.html
Created attachment 135123 [details] Patch DRT support for setTabKeyCyclesThroughElements
Comment on attachment 135123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135123&action=review > Tools/DumpRenderTree/efl/EventSender.cpp:464 > + Evas_Object* mainFrame = browser->mainFrame(); Missing const keyword. For example, const Evas_Object* mainFrame = browser->mainFrame(); > Tools/DumpRenderTree/efl/EventSender.cpp:468 > + evas_object_focus_set(mainFrame, EINA_TRUE); Use standard boolean type instead of EINA_TRUE.
Created attachment 135286 [details] Patch Fixed: EINA_TRUE -> true Unfortunately I can't make mainFrame const, since it is passed to setter and it expects non-const parameter.
Comment on attachment 135286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135286&action=review > Tools/DumpRenderTree/efl/EventSender.cpp:467 > + // Focus of the main frame could be lost during the test > + // Therefore, setting focus to the main frame Could you clarify this comment? What kind of scenario would cause the focus to be moved elsewhere? And what would this elsewhere be?
(In reply to comment #4) > (From update of attachment 135286 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135286&action=review > > > Tools/DumpRenderTree/efl/EventSender.cpp:467 > > + // Focus of the main frame could be lost during the test > > + // Therefore, setting focus to the main frame > > Could you clarify this comment? What kind of scenario would cause the focus to > be moved elsewhere? And what would this elsewhere be? There is 1 focusable node in a test case, which is an input element. In the beginning of a test case, we focus on an input element and then we emulate keypress "tab". Since there is no focusable element, focus controller will ask "chrome" to take focus. In case of EFL, whole view would be unfocused, all consecutive events would be omitted, since the view is unfocused. Similar "solution" is used in gtk DRT code.
Comment on attachment 135286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135286&action=review >>> Tools/DumpRenderTree/efl/EventSender.cpp:467 >>> + // Therefore, setting focus to the main frame >> >> Could you clarify this comment? What kind of scenario would cause the focus to be moved elsewhere? And what would this elsewhere be? > > There is 1 focusable node in a test case, which is an input element. In the beginning of a test case, we focus on an input element and then we emulate keypress "tab". Since there is no focusable element, focus controller will ask "chrome" to take focus. In case of EFL, whole view would be unfocused, all consecutive events would be omitted, since the view is unfocused. Similar "solution" is used in gtk DRT code. OK, looks fine then.
Created attachment 140916 [details] Patch Rebased to master
Comment on attachment 140916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140916&action=review do you need to "reset to default" before each test test? > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:468 > + WebCore::Page* page = EWKPrivate::corePage(ewkView); > + > + if (!page) bogus extra line
Created attachment 141008 [details] Patch Removed extra line
Created attachment 141014 [details] Patch Reset WebCore::Page::m_tabKeyCyclesThroughElements to default
(In reply to comment #8) > (From update of attachment 140916 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140916&action=review > > do you need to "reset to default" before each test test? Fixed > > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:468 > > + WebCore::Page* page = EWKPrivate::corePage(ewkView); > > + > > + if (!page) > > bogus extra line Fixed
Comment on attachment 141014 [details] Patch Clearing flags on attachment: 141014 Committed r116633: <http://trac.webkit.org/changeset/116633>
All reviewed patches have been landed. Closing bug.
New failures after this patch: fast/events/onchange-searchfield.html fast/events/check-defocus-event-order-when-triggered-by-mouse-click.html fast/events/check-defocus-event-order-when-triggered-by-tab.html fast/events/onchange-passwordfield.html fast/events/inputText-never-fired-on-keydown-cancel.html fast/events/onchange-select-popup.html fast/events/onchange-textfield.html fast/events/autoscroll-should-not-stop-on-keypress.html fast/forms/textfield-no-linebreak.html fast/spatial-navigation/snav-iframe-flattening-simple.html fast/spatial-navigation/snav-iframe-nested.html fast/spatial-navigation/snav-iframe-recursive-offset-parent.html fast/spatial-navigation/snav-iframe-with-offscreen-focusable-element.html
Re-opened since this is blocked by 86093
Created attachment 141171 [details] Patch Fixes event grabbing problem. Focus was set to the frame instead of the view.
Created attachment 141214 [details] Patch Updated changelog after rebasing
Created attachment 143260 [details] Patch Rebased to master
(In reply to comment #18) > Created an attachment (id=143260) [details] > Patch > > Rebased to master What is difference from r116633 ? It looks almost this patch was already landed by r116633. Was this patch roll out ? If so, it looks REOPEN is correct status.
Created attachment 144885 [details] Patch Rebased to master
Comment on attachment 144885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144885&action=review > Tools/DumpRenderTree/efl/EventSender.cpp:548 > + // Focus of the main frame could be lost during the test > + // Therefore, setting focus to the view > + evas_object_focus_set(view, true); Will this end up changing the focus away from a control? what does this set the focus to?
(In reply to comment #21) > (From update of attachment 144885 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144885&action=review > > > Tools/DumpRenderTree/efl/EventSender.cpp:548 > > + // Focus of the main frame could be lost during the test > > + // Therefore, setting focus to the view > > + evas_object_focus_set(view, true); > > Will this end up changing the focus away from a control? From HTML controls, no. >>what does this set the focus to? To the main efl view (ewk_view) When we cycle thru elements and last element is reached, in WebCore::FocusController::advanceFocusInDocumentOrder focus is passed to Chrome. Code snippet from focus controller: // We didn't find a node to focus, so we should try to pass focus to Chrome. m_page->chrome()->takeFocus(direction); in EFL port, takeFocus removes focus from the view void ChromeClientEfl::takeFocus(FocusDirection) { unfocus(); } In DRT context, focus is never set back to the view, therefore, all events are dismissed, since the view is unfocused. This patch forces canvas to be always in focused state, so that EventSender could send events to the view.
Comment on attachment 144885 [details] Patch Looks fine.
Comment on attachment 144885 [details] Patch Rejecting attachment 144885 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: LED -- saving rejects to file Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp.rej patching file Tools/DumpRenderTree/efl/EventSender.cpp patching file Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp Hunk #1 FAILED at 241. 1 out of 1 hunk FAILED -- saving rejects to file Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Gyuyoung K..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13739156
(In reply to comment #23) > (From update of attachment 144885 [details]) > Looks fine. Patch is quite old, I need to rebase it to master.
(In reply to comment #25) > (In reply to comment #23) > > (From update of attachment 144885 [details] [details]) > > Looks fine. > > Patch is quite old, I need to rebase it to master. Could you please check if this patch is still needed ?
This patch seems to address the efl version of DumpRenderTree, which no longer exists. I'm going to close it, but feel free to reopen it if something is needed for WebKitTestRunner.