w3c / uievents

UI Events
https://w3c.github.io/uievents/
Other
147 stars 52 forks source link

Click event is not fired when the node is removed and then added back #141

Open NavidZ opened 7 years ago

NavidZ commented 7 years ago

Looking at this example note in the spec it is not very explicit what should happen when a node is removed and then added back in the mousedown handler and we don't have any wpt test for it either. This example shows the behavior. Particularly it seems that Edge, Safari, and the latest version of Chrome don't send the click event but they do send both mousedown and up events to that element. FireFox however does send click event in that scenario. I can see that spec is implying that the click/dblclick events should not be sent to the node if it gets removed regardless of whether it gets added back or not. But it might not be very clear. @garykac @smaug---- @patrickkettner what do you think?

smaug---- commented 7 years ago

Do other browser fire click targeted to some ancestor?

Since mousedown is dispatched, click should be dispatched too. The spec talks about the case when element is not added back to the document.

Why did Chrome change its behavior?

NavidZ commented 7 years ago

No. None of the other browsers except FF send the click event even to any ancestor.

Regarding the mousedown and when an element is removed, just because a mousedown is there doesn't mean that there should also be a click. User might have changed the location to another iframe or other window and stuff like that. However, spec does say the mouseup should still be fired on the element that the mouse is on when the button is released. So I totally agree that the same element should also get a mouseup. Regarding the click I wonder since this is an activation signal and user gesture, if DOM is mutated then user might not have the intention of continuing that action anymore. So I can kind of buy not sending those kind of events if something right underneath the pointer is changing in this situation. WDYT? Adding @RByers who might be able to articulate this better.

Chrome didn't change the behavior in that case. Chrome always never fired click for the example mentioned in the comment. But we recently removed an optimization to match the spec and other vendors regarding DOM mutation functions to actually remove and add element in some of the function calls like node.appendChild(node.lastChild) and that caused this difference in behavior to surface in more scenarios. As with that non-standard optimization we didn't actually remove the node and still sent the click event. But now without that optimization and with the removal of the node there will be no click.

dtapuska commented 7 years ago

I do think that an end developer doesn't expect that performing an appendNode to move an item in DOM order amongst siblings (which is defined to do an adoptNode, which does a remove and then an insertion) a click event won't fire.

The fundamental principle is that Chrome, Edge, and Safari all seem to listen to DOM mutations and if the node is removed from the document the click node is cleared. Yet FireFox seems look at whether the node is connected to the document before firing the click.

@domenic Can anyone think of any experiments we could run? To try to figure out the interop risk of actually making such a change? I presume Chrome could add a histogram that tracks the times we encounter this scenario (where the click node is cleared yet the clicked node is still attached to the document). Although I don't know how we'd assess any interop impact.

domenic commented 7 years ago

This seems very related to work @annevk has been doing recently per https://github.com/whatwg/html/issues/2615, which includes some interop research.

annevk commented 7 years ago

Looking through this I don't think it's related. This bug is just a symptom of UI event dispatch being poorly defined.

Konard commented 5 years ago

Looks like this issue also applies the usage of the double click event. WebKit browsers stop fire event, FireFox continues. Example.

mustaqahmed commented 3 years ago

Since click is fired to the "nearest common inclusive ancestor" of mousedown and mouseup targets (see here), we need to redefine this term in the spec if we want to fire a click in this case.

If the mousedown target element is removed from DOM, it's ancestry in that DOM becomes undefined. Later on, if the same element is added back to the same DOM and it receives a mouseup, the "nearest common inclusive ancestor" of mousedown and mouseup remains undefined IMO. From this perspective, current Chrome and Safari behavior of not firing a click seems logical. I find it unreasonable to expect the removed element (or the DOM, or the event dispatcher) to somehow bring back the past ancestry information to find the "nearest common inclusive ancestor".

To highlight this point, here is another repro where the "moved element" changes its parent. Current Chrome and Safari behavior is in-line with DOM mutation algorithm (see more details in this Chrome bug comment), and I can't explain the click in Firefox!

smaug---- commented 3 years ago

It is hard to understand why Blink (nor webkit?) doesn't fire click. The mousedown and up happen on the same element, so click should fire there too. I don't understand what 'click node' https://github.com/w3c/uievents/issues/141#issuecomment-298918086 is talking about. The spec doesn't have such.

mustaqahmed commented 2 years ago

Here is Chrome's argument for not firing a click: by definition, the "moved child" is not connected to DOM for a while (see "Point 2" here), therefore the ancestry info of the mousedown target is logically reset after the move, making the common ancestor undefined.

bksglatz commented 2 years ago

I can understand why the click does not happen. But it feels wrong for me too. Me and other developers im working with would have expected a click because like @smaug---- has said, the mousedown and mouseup are on the same element. And at least Firefox seems to be able to handle this.