Focusing an <input> which focusing something else in its "onfocus" may trigger a extra, duplicate events.
Created attachment 65645 [details] [REDUCTION] Manual Test
Created attachment 65646 [details] [TEST] Automatic Test
Looks like there might be multiple dispatches stemming from EventHandler::handleMousePressEvent(const PlatformMouseEvent& mouseEvent). A look at the 4 backtraces (all 4 focus events fired) show this function as the root of the two dispatching branches: Breakpoint 1, WebCore::HTMLInputElement::handleFocusEvent (this=0x112917900) at WebCore/html/HTMLInputElement.cpp:728 728 InputElement::dispatchFocusEvent(this, this); #0 WebCore::HTMLInputElement::handleFocusEvent at WebCore/html/HTMLInputElement.cpp:728 #1 WebCore::HTMLTextFormControlElement::dispatchFocusEvent at WebCore/html/HTMLFormControlElement.cpp:487 #2 WebCore::Document::setFocusedNode at WebCore/dom/Document.cpp:3070 #3 WebCore::FocusController::setFocusedNode at WebCore/page/FocusController.cpp:627 #4 WebCore::EventHandler::dispatchMouseEvent at WebCore/page/EventHandler.cpp:1890 > #5 WebCore::EventHandler::handleMousePressEvent at WebCore/page/EventHandler.cpp:1305 #6 WebCore::EventHandler::mouseDown at WebCore/page/mac/EventHandlerMac.mm:494 #7 -[WebHTMLView at WebKit/mac/WebView/WebHTMLView.mm:3578 #8 0x00007fff8678434f in -[NSWindow sendEvent:] () #9 0x0000000100041889 in ?? () #10 0x0000000100041815 in ?? () #11 0x00007fff866b9a86 in -[NSApplication sendEvent:] () #12 0x000000010003855a in ?? () #13 0x00007fff866504da in -[NSApplication run] () #14 0x00007fff866491a8 in NSApplicationMain () #15 0x0000000100009804 in ?? () Breakpoint 1, WebCore::HTMLInputElement::handleFocusEvent (this=0x112917900) at WebCore/html/HTMLInputElement.cpp:728 728 InputElement::dispatchFocusEvent(this, this); #0 WebCore::HTMLInputElement::handleFocusEvent at WebCore/html/HTMLInputElement.cpp:728 #1 WebCore::HTMLTextFormControlElement::dispatchFocusEvent at WebCore/html/HTMLFormControlElement.cpp:487 #2 WebCore::Document::setFocusedNode at WebCore/dom/Document.cpp:3070 #3 WebCore::FocusController::setFocusedNode at WebCore/page/FocusController.cpp:627 #4 WebCore::Frame::setFocusedNodeIfNeeded at WebCore/page/Frame.cpp:592 #5 WebCore::SelectionController::setSelection at WebCore/editing/SelectionController.cpp:156 #6 WebCore::SelectionController::setSelection at SelectionController.h:68 #7 WebCore::EventHandler::handleMousePressEventSingleClick at WebCore/page/EventHandler.cpp:403 #8 WebCore::EventHandler::handleMousePressEvent at WebCore/page/EventHandler.cpp:472 > #9 WebCore::EventHandler::handleMousePressEvent at WebCore/page/EventHandler.cpp:1346 #10 WebCore::EventHandler::mouseDown at WebCore/page/mac/EventHandlerMac.mm:494 #11 -[WebHTMLView at WebKit/mac/WebView/WebHTMLView.mm:3578 #12 0x00007fff8678434f in -[NSWindow sendEvent:] () #13 0x0000000100041889 in ?? () #14 0x0000000100041815 in ?? () #15 0x00007fff866b9a86 in -[NSApplication sendEvent:] () #16 0x000000010003855a in ?? () #17 0x00007fff866504da in -[NSApplication run] () #18 0x00007fff866491a8 in NSApplicationMain () #19 0x0000000100009804 in ?? () Where the lines are: (1) bool swallowEvent = dispatchMouseEvent(eventNames().mousedownEvent, mev.targetNode(), true, m_clickCount, mouseEvent, true); (2) swallowEvent = handleMousePressEvent(mev); Note that (2) is called expecting swallowEvent to have been false. Maybe (1) should have returned true? I'll be investigating.
Duplicate events also happen when tabbing to the input, not just via mousePress. This makes me think there is a more generic problem. It seems to me that if the focus changed during the dispatchEvent, than later on during the setSelection path, the node should not dispatch a focus event. Or maybe it should never at all. I have to learn more about what is going on. Breakpoint 1, WebCore::HTMLInputElement::handleFocusEvent (this=0x1124321e0) at WebCore/html/HTMLInputElement.cpp:728 (gdb) bt #0 WebCore::HTMLInputElement::handleFocusEvent at WebCore/html/HTMLInputElement.cpp:728 #1 WebCore::HTMLTextFormControlElement::dispatchFocusEvent at WebCore/html/HTMLFormControlElement.cpp:487 #2 WebCore::Document::setFocusedNode at WebCore/dom/Document.cpp:3070 #3 WebCore::FocusController::setFocusedNode at WebCore/page/FocusController.cpp:627 > #4 WebCore::Element::focus at WebCore/dom/Element.cpp:1317 #5 WebCore::FocusController::advanceFocusInDocumentOrder at WebCore/page/FocusController.cpp:285 #6 WebCore::FocusController::advanceFocus at WebCore/page/FocusController.cpp:170 #7 WebCore::EventHandler::defaultTabEventHandler at WebCore/page/EventHandler.cpp:2750 #8 WebCore::EventHandler::defaultKeyboardEventHandler at WebCore/page/EventHandler.cpp:2388 #9 WebCore::Node::defaultEventHandler at WebCore/dom/Node.cpp:3018 #10 WebCore::HTMLFormControlElementWithState::defaultEventHandler at WebCore/html/HTMLFormControlElement.cpp:471 #11 WebCore::HTMLInputElement::defaultEventHandler at WebCore/html/HTMLInputElement.cpp:2177 #12 WebCore::Node::dispatchGenericEvent at WebCore/dom/Node.cpp:2747 #13 WebCore::Node::dispatchEvent at WebCore/dom/Node.cpp:2631 #14 WebCore::EventTarget::dispatchEvent at WebCore/dom/EventTarget.cpp:278 #15 WebCore::EventHandler::keyEvent at WebCore/page/EventHandler.cpp:2325 #16 WebCore::EventHandler::keyEvent at WebCore/page/mac/EventHandlerMac.mm:148 #17 -[WebHTMLView at WebKit/mac/WebView/WebHTMLView.mm:4097 #18 0x00007fff8678506f in -[NSWindow sendEvent:] () Breakpoint 1, WebCore::HTMLInputElement::handleFocusEvent (this=0x1124321e0) at WebCore/html/HTMLInputElement.cpp:728 (gdb) bt #0 WebCore::HTMLInputElement::handleFocusEvent at WebCore/html/HTMLInputElement.cpp:728 #1 WebCore::HTMLTextFormControlElement::dispatchFocusEvent at WebCore/html/HTMLFormControlElement.cpp:487 #2 WebCore::Document::setFocusedNode at WebCore/dom/Document.cpp:3070 #3 WebCore::FocusController::setFocusedNode at WebCore/page/FocusController.cpp:627 #4 WebCore::Frame::setFocusedNodeIfNeeded at WebCore/page/Frame.cpp:592 #5 WebCore::SelectionController::setSelection at WebCore/editing/SelectionController.cpp:156 #6 WebCore::RenderTextControl::setSelectionRange at WebCore/rendering/RenderTextControl.cpp:262 #7 WebCore::RenderTextControl::select at WebCore/rendering/RenderTextControl.cpp:233 #8 WebCore::HTMLTextFormControlElement::select at WebCore/html/HTMLFormControlElement.cpp:564 #9 WebCore::HTMLInputElement::select at HTMLInputElement.h:146 #10 WebCore::InputElement::updateFocusAppearance at WebCore/dom/InputElement.cpp:95 #11 WebCore::HTMLInputElement::updateFocusAppearance at WebCore/html/HTMLInputElement.cpp:707 > #12 WebCore::Element::focus at WebCore/dom/Element.cpp:1329 #13 WebCore::FocusController::advanceFocusInDocumentOrder at WebCore/page/FocusController.cpp:285 #14 WebCore::FocusController::advanceFocus at WebCore/page/FocusController.cpp:170 #15 WebCore::EventHandler::defaultTabEventHandler at WebCore/page/EventHandler.cpp:2750 #16 WebCore::EventHandler::defaultKeyboardEventHandler at WebCore/page/EventHandler.cpp:2388 #17 WebCore::Node::defaultEventHandler at WebCore/dom/Node.cpp:3018 #18 WebCore::HTMLFormControlElementWithState::defaultEventHandler at WebCore/html/HTMLFormControlElement.cpp:471 #19 WebCore::HTMLInputElement::defaultEventHandler at WebCore/html/HTMLInputElement.cpp:2177 #20 WebCore::Node::dispatchGenericEvent at WebCore/dom/Node.cpp:2747 #21 WebCore::Node::dispatchEvent at WebCore/dom/Node.cpp:2631 #22 WebCore::EventTarget::dispatchEvent at WebCore/dom/EventTarget.cpp:278 #23 WebCore::EventHandler::keyEvent at WebCore/page/EventHandler.cpp:2325 #24 WebCore::EventHandler::keyEvent at WebCore/page/mac/EventHandlerMac.mm:148 #25 -[WebHTMLView at WebKit/mac/WebView/WebHTMLView.mm:4097 #26 0x00007fff8678506f in -[NSWindow sendEvent:] ()
Okay, all calls to FocusController::setFocusedNode(n) and Document::setFocusedNode(n) may return with the focus node not being (n). Document::setFocusedNode returns false in this case. FocusController::setFocusedNode may return true in this case. But both are commonly used without checking their return value, and I think FocusController's return value is wrong in the above cases.
Created attachment 65776 [details] [PATCH] Respect setFocusedNode return values This is not an area I am very familiar with, so I would appreciate a careful review.
Committed r66284 M WebCore/dom/Element.cpp M WebCore/ChangeLog M WebCore/page/FocusController.cpp A LayoutTests/fast/events/input-tab-focus-no-duplicate-events-expected.txt A LayoutTests/fast/events/input-focus-no-duplicate-events.html A LayoutTests/fast/events/input-tab-focus-no-duplicate-events.html A LayoutTests/fast/events/input-focus-no-duplicate-events-expected.txt M LayoutTests/ChangeLog r66284 = 53ef8428545a26d93c3cc6bf22e51eed368403a7 (refs/remotes/trunk) http://trac.webkit.org/changeset/66284 Watching the bots.
Attachment 65776 [details] was posted by a committer and has review+, assigning to Joseph Pecoraro for commit.
Whoops, I posted had landed this but forgot to close. Thanks for the reminder.
Me has horrible english... I landed this in r66284. ;)