Able to reproduce on Windows-10, Ubuntu 14.04 and Mac OS 10.12.4 using chrome stable #57.0.2987.133

Bisect Information:
=====================
Good build: 57.0.2938.0
Bad Build : 57.0.2940.0

Change Log URL:
https://chromium.googlesource.com/chromium/src/+log/4f7bf8250fd4c45f5430d09f76e581acb24000b4..946f30141fad7e002a44d3545a7d9f45e3cdcbf3 

From the above change log suspecting below change
Review URL: https://codereview.chromium.org/2532393003

hayato@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Sure, I'll take a look tomorrow.

A friendly reminder that M58 Stable launch is coming soon! Your bug is labelled as Stable ReleaseBlock, please make sure to land the fix, verified in trunk and get it merged into the release branch ASAP.

I can reproduce this, however, I rather would mark this WONTFIX for the following reasons:

- This looks a correct behavior, in terms of spec-conformance:

  The order of events related to the onmouseup event:
    1. onmousedown
    2. onmouseup
    3. onclick
- The current behavior matches the behavior of firefox 52.0.1. (Disclaimer: I have not confirmed the behavior of edge nor safari)

I am not 100% sure how big impact this *regression* would have in the wild.
Please re-open this if someone have any concern.

This works in Edge, IE11, IE10, and Safari 9.

https://www.w3.org/TR/uievents/#events-mouseevent-event-order says:

"Each implementation will determine the appropriate hysteresis tolerance, but in general SHOULD fire click and dblclick events when the event target of the associated mousedown and mouseup events is the same element with no mouseout or mouseleave events intervening".

In this case I can't see how we would be outside of any "hysteresis tolerance".  The "mousedown" and "mouseup" events are occurring on the exact same target element (with no mouseout or mouseleave events in between).

I'll let others decide on the "SHOULD" and impact this will have in the wild. 

No, that is not relevant, I think.

In the next paragraph, the spec says:

> If the event target (e.g. the target element) is removed from the DOM during the mouse events sequence, the remaining events of the sequence MUST NOT be fired on that element.

hayato, we should fix this according to the specification.

https://www.w3.org/TR/uievents/#event-type-click
> The click event type MUST be dispatched on the topmost	event target indicated by the pointer, when the user presses down and releases the primary pointer button, 

https://www.w3.org/TR/uievents/#topmost-event-target
> The topmost event target MUST be the **element** highest in the rendering order which is capable of being an event target.


In the test case, topmost event target should be <div id="BL2">, and mousedown and mouseup happen on it.

tkent,

I think that is not relevant because click event's target is determined before mosueup event is fired. I think that is what spec is assuming, as far as I can read from:

(https://w3c.github.io/uievents/#events-mouseevent-event-order)
If the target element is removed from the DOM as the result of a mousedown event, no events for that element will be dispatched	for mouseup, click, or dblclick, nor any default	activation events. However, the mouseup event will still be	dispatched on the element that is exposed to the mouse after the removal of the initial target element. Similarly, if the target element is removed from the DOM during the dispatch of a mouseup event, the click and subsequent events will not be dispatched.

Ah, I understand the intention, you meant "id="BL2" element, which is the parent of a text node. Please ignore comment #9.

hayato, the target element isn't removed from the tree in the test case though a Text child is replaced.

Yeah, I understand it now. Let me re-open this since the text node should not be a click event's target, as far as I can read from the spec, as tkent@ pointed out.

Further test to conclude that the target element is not being replaced. 

Deleted: missing_onclick2.html 1.9 KB

missing_onclick2.html 1.9 KB View Download

Do we have an ETA(tentative) when this fix shows up?

Let me work on this today.

I've investigated this issue. The CL [1] is not the direct root cause.
Actually, a test case [2], which removes a text node without using innerHTML, *FAILS* regardless of the CL.

It looks that there were *two* bugs related to this issue in Blink:

1. One is the bug which the CL [1] fixed.
2. The other is here [3]; if commonAncestor of click_node and men.innerNode() (which would be a text node) is null, Blink does not dispatch a click event. That is the reason why [2] fails.

Unfortunately, these two bugs acted as *double negative*, letting Blink behave as *partially-positive*.
The CL [1] fixed one bug (*flipping* one *negative*), making double-negative to single-negative, and revealed the wrong behavior. We have to fix 2, as well as 1.

I will continue to work on fixing the root cause, 2, however, let me remove Release-Blocker label here tentatively
because [2], which might be the majority case of bugs, is not a regression.

[1] https://codereview.chromium.org/2532393003
[2] https://jsfiddle.net/hayatoito/nkax6f8v/
[3] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/input/MouseEventManager.cpp?l=270

WIP CL is here: https://codereview.chromium.org/2807123002

The users of our product (IBM Forms Experience Builder) are being affected by this bug. Please let us know what the priority and estimated time is for having it rolled out to the public.

To be clear on the impact to IBM and our product; this bug does limit basic functionality of current deployments of IBM Forms Experience Builder. This is potentially effecting thousands of end-users and hundreds of companies who host their own on-premise instance of our product. It is difficult to roll out a workaround to all these customers. We would urge that this bug is given high priority and considered seriously for the next roll-out of Chrome. 

rlintenr@, thank you for letting us know. Let me mark this as P1.

Since it might take some time to fix the root cause, as you might notice in https://codereview.chromium.org/2807123002, I am tempting that I will land a tentative fix as I did in https://codereview.chromium.org/2807123002/#ps1.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a971919aa2691e65eaf9a085b949ed6effc143a3

commit a971919aa2691e65eaf9a085b949ed6effc143a3
Author: hayato <hayato@chromium.org>
Date: Wed Apr 12 07:52:45 2017

Fire a click event even when a clicked text node is removed in mouseup

This is a tentative fix for  bug 708394 .
See also https://codereview.chromium.org/2807123002/ for details, which is trying
to fix the root cause. Since fixing the root cause might change the other
behaviors as well as fixing  bug 708394 , let me land the tentative fix to prevent
a regression in M58.

Blink does not fire a click event if a user removes a clicked text node in mouseup
event listener because of the followings:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this case.
   - A location of mouse released; The 2nd hit test's result is used in this case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

This CL keeps the parent element, if necessary, of 2nd hit test's result before
dispatching a mouseup event, and use it to decide whether to dispatch a click
event or not.

BUG= 708394 

Review-Url: https://codereview.chromium.org/2812613004
Cr-Commit-Position: refs/heads/master@{#463950}

[add] https://crrev.com/a971919aa2691e65eaf9a085b949ed6effc143a3/third_party/WebKit/LayoutTests/fast/events/remove-text-node-in-mouseup-expected.txt
[add] https://crrev.com/a971919aa2691e65eaf9a085b949ed6effc143a3/third_party/WebKit/LayoutTests/fast/events/remove-text-node-in-mouseup.html
[modify] https://crrev.com/a971919aa2691e65eaf9a085b949ed6effc143a3/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/a971919aa2691e65eaf9a085b949ed6effc143a3/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/a971919aa2691e65eaf9a085b949ed6effc143a3/third_party/WebKit/Source/core/input/MouseEventManager.h

Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Ah, I should have waited for a canary build before requesting Merge-Request-58.
Let me merge the CL after the CL passed a canary build.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a71136c3bad4746d6d66a4628985640895659138

commit a71136c3bad4746d6d66a4628985640895659138
Author: Hayato Ito <hayato@google.com>
Date: Thu Apr 13 04:14:08 2017

Fire a click event even when a clicked text node is removed in mouseup

This is a tentative fix for  bug 708394 .
See also https://codereview.chromium.org/2807123002/ for details, which is trying
to fix the root cause. Since fixing the root cause might change the other
behaviors as well as fixing  bug 708394 , let me land the tentative fix to prevent
a regression in M58.

Blink does not fire a click event if a user removes a clicked text node in mouseup
event listener because of the followings:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this case.
   - A location of mouse released; The 2nd hit test's result is used in this case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

This CL keeps the parent element, if necessary, of 2nd hit test's result before
dispatching a mouseup event, and use it to decide whether to dispatch a click
event or not.

BUG= 708394 

Review-Url: https://codereview.chromium.org/2812613004
Cr-Commit-Position: refs/heads/master@{#463950}
(cherry picked from commit a971919aa2691e65eaf9a085b949ed6effc143a3)

Review-Url: https://codereview.chromium.org/2814343002 .
Cr-Commit-Position: refs/branch-heads/3029@{#688}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[add] https://crrev.com/a71136c3bad4746d6d66a4628985640895659138/third_party/WebKit/LayoutTests/fast/events/remove-text-node-in-mouseup-expected.txt
[add] https://crrev.com/a71136c3bad4746d6d66a4628985640895659138/third_party/WebKit/LayoutTests/fast/events/remove-text-node-in-mouseup.html
[modify] https://crrev.com/a71136c3bad4746d6d66a4628985640895659138/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/a71136c3bad4746d6d66a4628985640895659138/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/a71136c3bad4746d6d66a4628985640895659138/third_party/WebKit/Source/core/input/MouseEventManager.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6

commit 85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6
Author: hayato <hayato@chromium.org>
Date: Thu Apr 13 08:53:14 2017

Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, at which an event should be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG= 708394 ,710425

Review-Url: https://codereview.chromium.org/2807123002
Cr-Commit-Position: refs/heads/master@{#464344}

[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/LayoutTests/fast/events/touch/gesture/gesture-tap-frame-scrollbar-expected.txt
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/EventHandlingUtil.cpp
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/EventHandlingUtil.h
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/GestureManager.cpp
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6/third_party/WebKit/Source/core/input/MouseEventManager.h

Verified on Chrome:58.0.3029.76 Device:Moto Z(XT1650)/NPLS25/7.0.0

Verfied on Chrome:59.0.3071.8 Device:Moto Z(XT1650)/NPLS25/7.0.0

Verified the issue on windows 7, Mac 10.12.4 and Ubuntu 14.04 using Chrome version #58.0.3029.81 as per comment #0.

Observed that the fix is working as expected.Hence adding the verified labels.

Please find the attached screen cast for reference.

Thanks.

Deleted: 708394.mp4 174 KB

	708394.mp4 174 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14bcd90c3e83b60fb9ecce8504356b81af4c6209

commit 14bcd90c3e83b60fb9ecce8504356b81af4c6209
Author: hayato <hayato@chromium.org>
Date: Wed Apr 19 08:54:54 2017

Remove the dup function

The context: https://codereview.chromium.org/2807123002#msg83

Use HitTestResult::innerElement, instead of adding EventHandlerUtil::ParentElementIfNeeded().

BUG= 708394 ,710425

Review-Url: https://codereview.chromium.org/2829553002
Cr-Commit-Position: refs/heads/master@{#465536}

[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/input/EventHandlingUtil.cpp
[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/input/EventHandlingUtil.h
[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/input/GestureManager.cpp
[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/layout/HitTestResult.cpp
[modify] https://crrev.com/14bcd90c3e83b60fb9ecce8504356b81af4c6209/third_party/WebKit/Source/core/page/EventWithHitTestResults.h