Bug 39620 - Clicks inside button elements are sometimes discarded when the mouse moves
: Clicks inside button elements are sometimes discarded when the mouse moves
Status: NEW
Product: WebKit
Classification: Unclassified
Component: Forms
: 528+ (Nightly build)
: Macintosh Intel Mac OS X 10.6
: P2 Normal
Assigned To: Nobody
: NeedsReduction
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-24 14:42 PDT by bryan
Modified: 2013-11-28 15:31 PST (History)
19 users (show)

See Also:


Attachments
image of a button with inner text box (3.95 KB, image/png)
2010-05-24 14:42 PDT, bryan
no flags Details
Simple Page which demostrates this bug. (831 bytes, text/html)
2011-04-08 00:02 PDT, ted1
no flags Details
work in progress (4.02 KB, patch)
2011-05-11 07:41 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description bryan 2010-05-24 14:42:11 PDT
Created attachment 56929 [details]
image of a button with inner text box

When a click originates inside one element of a button (e.g., the button text) and terminates outside of that element (e.g., another element, or just the button box) the onClick() method isn't triggered.

This is reproducible on websites like facebook.  You can reproduce by clicking inside the text box, holding the mouse down, and then releasing the mouse button outside of the text box, but still inside the button.  While the facebook buttons are styled such that this doesn't happen by accident all that often, buttons with slightly more padding cause this issue frequently.

Both safari and chrome exhibit this behavior, but firefox does not.

Attached is a button from facebook that shows the inner text box inside the larger button box.  A click that Goes from inside-to-outside will not register.
Comment 1 Alexey Proskuryakov 2010-05-24 22:07:25 PDT
Someone needs to grab this bit of HTML/CSS code to investigate.
Comment 2 ted1 2011-04-08 00:02:54 PDT
Created attachment 88775 [details]
Simple Page which demostrates this bug.

This pages shows the bug. It works correctly in Firefox 3.6.
Comment 3 ted1 2011-04-08 00:05:45 PDT
I hit this bug, too. I'm using QtWebKit shipped with Qt 4.7.2 on Windows XP. This bug also affects Safari 5.0.4 on Windows XP.

I stepped through the QtWebKit code. No click event is generated in webkit\WebCore\page\EventHandler.cpp because mev.targetNode() != m_clickNode in the function 
bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
Comment 4 Ryosuke Niwa 2011-04-11 13:33:08 PDT
Confirmed on Nightly WebKit. The bug does not reproduce on Firefox 4.
Comment 5 Kent Tamura 2011-04-11 19:59:32 PDT
IE8 behaves as same as Firefox.
Ok, we should fix this.
Comment 6 Ryosuke Niwa 2011-04-14 17:10:13 PDT
"5.2.3.2. Mouse Event Order" (http://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevent-event-order) has the following statement:

in general should fire click anddblclick events when the event target of the associated mousedown and mouseup events is the same element with no mouseout or mouseleave events intervening, and should not fire click and dblclick events when the event target of the associated mousedown and mouseup events is different.

So current behavior of WebKit follows the spec. Firefox avoids this bug by firing all mousedown, mouseup, and click on button element and Internet Explorer always fires click regardless of whether mouseup/mousedown's targets match each other.
Comment 7 Alexey Proskuryakov 2011-04-14 17:20:23 PDT
User experience is definitely wrong here, so this is something that needs to be fixed one way or another.
Comment 8 bryan 2011-04-14 17:21:37 PDT
I guess the question is whether text inside a button is a separate element from the button, or part of the button.  I think common sense would say that any text in a button is logically part of the button itself.
Comment 9 Ryosuke Niwa 2011-04-14 17:50:19 PDT
Per discussion on IRC, I sent an email to www-dom: http://lists.w3.org/Archives/Public/www-dom/2011AprJun/0025.html

We should wait fixing this bug until some consensus is reached there.
Comment 10 Ryosuke Niwa 2011-04-14 17:53:06 PDT
I'm going to assign the bug to myself at least until I get some response on www-dom.
Comment 11 Ryosuke Niwa 2011-05-11 07:41:34 PDT
Created attachment 93116 [details]
work in progress

In this patch, I'm dispatching click event on the least common ancestor of clickNode and the target node for mouseup event.  I'm not sure this is the correct approach given that IE and FF both special cases button element.
Comment 12 Dimitri Glazkov (Google) 2011-05-11 09:59:40 PDT
I think we can reason about this using shadow DOM as a primitive:

1) Inside shadow DOM, mouseover/out events only escape the shadow DOM boundary if the relatedTarget of an event is outside of the shadow DOM boundary;

2) if we pretend that a button is a shadow DOM boundary, then the user should not hear mouseovers/outs when moving the mouse inside the button.

3) Since the events are retargeted to the outside of the shadow boundary, the effective target of the mouseup and mousedown events is always going to be the button element.

4) If so, the 5.2.3.2. section that Niwa-san mentioned makes perfect sense, relative to the outside of the shadow DOM boundary.

From this, it appears that click and dblclick events are relative in terms of the shadow DOM:

* Inside of the shadow DOM boundaries, the mousedown, mouseover, mouseout, and mouseup are all separate events.

* Outside of the shadow DOM boundary, they appear as part of a click/dblclick event sequence.

As an aside, this is a good indication that the current click/dblclick logic should probably move down from EventHandler, and into EventDispatcher, since only EventDispatcher is aware of the shadow DOM boundaries.
Comment 13 Ryosuke Niwa 2011-05-11 10:40:52 PDT
(In reply to comment #12)
> 1) Inside shadow DOM, mouseover/out events only escape the shadow DOM boundary if the relatedTarget of an event is outside of the shadow DOM boundary;

What is a related target?

> 2) if we pretend that a button is a shadow DOM boundary, then the user should not hear mouseovers/outs when moving the mouse inside the button.
> 
> 3) Since the events are retargeted to the outside of the shadow boundary, the effective target of the mouseup and mousedown events is always going to be the button element.
> 
> 4) If so, the 5.2.3.2. section that Niwa-san mentioned makes perfect sense, relative to the outside of the shadow DOM boundary.

However, a button doesn't have a shadow DOM. So I have a problem with the premise in point 2.
Comment 14 Dimitri Glazkov (Google) 2011-05-11 10:50:35 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > 1) Inside shadow DOM, mouseover/out events only escape the shadow DOM boundary if the relatedTarget of an event is outside of the shadow DOM boundary;
> 
> What is a related target?

https://developer.mozilla.org/en/DOM/event.relatedTarget

> 
> > 2) if we pretend that a button is a shadow DOM boundary, then the user should not hear mouseovers/outs when moving the mouse inside the button.
> > 
> > 3) Since the events are retargeted to the outside of the shadow boundary, the effective target of the mouseup and mousedown events is always going to be the button element.
> > 
> > 4) If so, the 5.2.3.2. section that Niwa-san mentioned makes perfect sense, relative to the outside of the shadow DOM boundary.
> 
> However, a button doesn't have a shadow DOM. So I have a problem with the premise in point 2.

Right, it currently doesn't. But it seems to fit pretty well, don't you think?
Comment 15 Ryosuke Niwa 2011-05-11 10:55:36 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > However, a button doesn't have a shadow DOM. So I have a problem with the premise in point 2.
> 
> Right, it currently doesn't. But it seems to fit pretty well, don't you think?

It really does. Only if we had a type of shadow host that acted like a normal node to JavaScript but acted like other shadow host inside WebCore...
Comment 16 Dimitri Glazkov (Google) 2011-05-11 11:00:31 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > However, a button doesn't have a shadow DOM. So I have a problem with the premise in point 2.
> > 
> > Right, it currently doesn't. But it seems to fit pretty well, don't you think?
> 
> It really does. Only if we had a type of shadow host that acted like a normal node to JavaScript but acted like other shadow host inside WebCore...

That's what <output> does. A button element is essentially this:

<template><output></output></template>
Comment 17 Ojan Vafai 2011-06-02 16:34:56 PDT
I think for the purposes of getting the correct user-experience and for web-compat it is sufficient to fire the click event on the deepest common ancestor. We should start there. If we need to match other browsers behavior by special-casing button elements, we can always do so later.
Comment 18 Ryosuke Niwa 2011-08-15 14:19:04 PDT
*** Bug 49297 has been marked as a duplicate of this bug. ***
Comment 19 Brock Williams 2012-05-09 09:11:21 PDT
This same bug also happens on Links.  We have a QtWebKit based touchscreen app, and we have added a large padding to all link tags to give a larger target for tapping.  If the user starts the tap on the link text and finishes over the padding, the click event does not fire.  I can confirm that the patch attached fixes the issue for us on both links and button elements.
Comment 20 Dominic Cooney 2012-12-16 18:32:22 PST
I am unblocking this from bug 56211. From Comment 12, it sounds like implementing button to use Shadow DOM (ie bug 56211) would fix this, which indicates bug 56211 blocks this bug.

However given Comment 19 it seems this applies to elements where there's no Shadow DOM in question (ie spans).
Comment 21 ernests 2013-03-28 23:34:16 PDT
*** Bug 113504 has been marked as a duplicate of this bug. ***
Comment 22 Alexander Shalamov 2013-04-11 23:17:34 PDT
*** Bug 114417 has been marked as a duplicate of this bug. ***
Comment 23 hugh.lomas 2013-11-12 10:32:54 PST
The priority on this needs to be increased, it is beyond the pale that a fundamental component of the browser is so horribly broken three years after being reported, and with a clear reproducible test case available. The example attachment https://bugs.webkit.org/attachment.cgi?id=88775 perfectly illustrates the problem. 

The *buttons* are not registering *clicks* in a very common occurrence: the mouse moving a little when clicking. The sole purpose, the reason buttons exist, is for clicking. 

What the fuck, honestly?
Comment 24 Jack O'Connor 2013-11-28 12:20:52 PST
This is a very serious issue if the button gets transformed down 1px on click. In this case the bug no longer relies on the user accidentally dragging during the click - it happens when the user does a normal click at the very top or bottom of the text. I have explained the problem in this codepen: http://codepen.io/jackocnr/pen/yLFhD
Comment 25 Kent Tamura 2013-11-28 15:31:39 PST
FYI. We recently fix this problem in Blink. http://src.chromium.org/viewvc/blink?view=revision&revision=162081