|Summary:||Click event listener remains registered after onclick attribute is removed|
|Product:||WebKit||Reporter:||Cary Clark <caryclark>|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Cary Clark 2009-01-14 08:21:00 PST
The attached test removes the onClick() attribute on the second element when the first element is clicked. The test works correctly -- the second element no longer responds to the click when the onClick() element is removed. However, the EventTargetNode RegisteredEventListenerList still includes an entry for the click event. Android iterates through all nodes looking for ones that receive clicks, and computes focus rings for those nodes. Attached is a patch that removes the event listener when the attribute is removed. The patch works, but isn't elegant. What's the right approach to fixing this bug?
Comment 1 Cary Clark 2009-01-14 08:23:07 PST
Created attachment 26709 [details] demonstration of bug
Comment 2 Cary Clark 2009-01-14 08:26:07 PST
Created attachment 26710 [details] proof of concept patch -- not intended to be landed This patch fixes the problem for Android -- but what's the right way to do it?
Comment 3 Darin Adler 2009-01-14 09:32:55 PST
Comment on attachment 26710 [details] proof of concept patch -- not intended to be landed The first thing we need here is a test case to demonstrate the problem. The details of the problem could be different with different node types, for example, with the window. And the failure may be caused by some other code change or some port-specific issue. A test case cuts through all those issues and lets everyone see that something is really wrong. The function responsible for handling this sort of thing is EventTargetNode::removeInlineEventListenerForType. In the failing test case, you should figure out if the problem is that function is not being called, or if the that function is not working properly. Specifically, if you change the value of the onclick attribute on a <button> element to the empty string (to choose a specific example, because there are many different code paths), then: EventTargetNode::setOnclick will call EventTargetNode::setInlineEventListenerForType, which will call EventTargetNode::removeInlineEventListenerForType, which will remove the listener that was added
Comment 4 Darin Adler 2009-01-14 09:35:04 PST
Comment 5 Darin Adler 2009-01-14 09:38:19 PST
(In reply to comment #4) > If you do removeAttribute, then it's different. I'll add comments about that > now. I'm almost certain that the bug here boils down to the fact that Element::removeAttribute doesn't call attributeChanged the way Element::setAttribute does. The attributeChanged function calls parseMappedAttribute, and then HTMLElement::parseMappedAttribute calls setInlineEventListenerForTypeAndAttribute. I see now the test case here. There's probably a much simpler way to demonstrate this problem that doesn't require user intervention.
Comment 7 Alexey Proskuryakov 2009-09-22 10:39:18 PDT
Looks like this issue only affects Android currently. But once Web Inspector begins to show registered event listeners, it will likely be affected.