Bug 44731 – Extra Events triggered by <input> on focus

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joseph Pecoraro]

Focusing an <input> which focusing something else in its "onfocus" may
trigger a extra, duplicate events.

[Joseph Pecoraro]

Created attachment 65645 [details]
[REDUCTION] Manual Test

[Joseph Pecoraro]

Created attachment 65646 [details]
[TEST] Automatic Test

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

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:] ()

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

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.

[Eric Seidel (no email)]

Attachment 65776 [details] was posted by a committer and has review+, assigning to Joseph Pecoraro for commit.

[Joseph Pecoraro]

Whoops, I posted had landed this but forgot to close. Thanks for the reminder.

[Joseph Pecoraro]

Me has horrible english...
I landed this in r66284. ;)