Changeset 244267 in webkit

Timestamp:

Apr 15, 2019, 10:26:39 AM (7 years ago)

Author:

Devin Rousso

Message:

Web Inspector: Elements: event listener change events should only be fired for the selected node and it's ancestors
​https://bugs.webkit.org/show_bug.cgi?id=196887
<rdar://problem/49870627>

Reviewed by Timothy Hatcher.

Source/WebCore:

Test: inspector/dom/event-listener-add-remove.html

inspector/dom/event-listener-inspected-node.html

• inspector/agents/InspectorDOMAgent.h:
• inspector/agents/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::getEventListenersForNode):
(WebCore::InspectorDOMAgent::setInspectedNode):
(WebCore::InspectorDOMAgent::didAddEventListener):
(WebCore::InspectorDOMAgent::willRemoveEventListener):

Source/WebInspectorUI:

• UserInterface/Models/DOMNode.js:

(WI.DOMNode.prototype.getEventListeners):

LayoutTests:

• inspector/dom/event-listener-inspected-node.html: Added.
• inspector/dom/event-listener-inspected-node-expected.txt: Added.

• inspector/dom/event-listener-add-remove.html:
• inspector/dom/event-listener-add-remove-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/dom/event-listener-add-remove-expected.txt (modified) (1 diff)
• LayoutTests/inspector/dom/event-listener-add-remove.html (modified) (3 diffs)
• LayoutTests/inspector/dom/event-listener-inspected-node-expected.txt (added)
• LayoutTests/inspector/dom/event-listener-inspected-node.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/inspector/agents/InspectorDOMAgent.cpp (modified) (5 diffs)
• Source/WebCore/inspector/agents/InspectorDOMAgent.h (modified) (2 diffs)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/Models/DOMNode.js (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-04-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Elements: event listener change events should only be fired for the selected node and it's ancestors
+        https://bugs.webkit.org/show_bug.cgi?id=196887
+        <rdar://problem/49870627>
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector/dom/event-listener-inspected-node.html: Added.
+        * inspector/dom/event-listener-inspected-node-expected.txt: Added.
+
+        * inspector/dom/event-listener-add-remove.html:
+        * inspector/dom/event-listener-add-remove-expected.txt:
+
 2019-04-15  Shawn Roberts  <sroberts@apple.com>
 

trunk/LayoutTests/inspector/dom/event-listener-add-remove-expected.txt

@@ -1 +1 @@
 Testing events for adding and removing event listeners.
 
+Changing inspected node to #document...
 
 == Running test suite: DOM.eventListeners

trunk/LayoutTests/inspector/dom/event-listener-add-remove.html

@@ -210 +210 @@
                         addAttribute("onload", 2);
                     });
-                } else if (eventCount === 2)
+                } else if (eventCount === 2) {
                     InspectorTest.log("Attribute event listener replaced.");
-                else if (eventCount === 3) {
+
                     logListeners(1).then(() => {
                         InspectorTest.log("");
@@ -218 +218 @@
                         removeAttribute("onload");
                     });
-                } else if (eventCount === 4)
+                } else if (eventCount === 3)
                     logListeners(0).then(resolve, reject);
                 else
@@ -228 +228 @@
     });
 
-    WI.domManager.requestDocument((documentNode) => {
+    WI.domManager.requestDocument(async (documentNode) => {
         node = documentNode;
+
+        InspectorTest.assert(node !== WI.domManager.inspectedNode);
+        InspectorTest.log("Changing inspected node to #document...");
+        await Promise.all([
+            WI.domManager.awaitEvent(WI.DOMManager.Event.InspectedNodeChanged),
+            WI.domManager.setInspectedNode(node),
+        ]);
+        InspectorTest.assert(node === WI.domManager.inspectedNode);
 
         suite.runTestCasesAndFinish();

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-04-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Elements: event listener change events should only be fired for the selected node and it's ancestors
+        https://bugs.webkit.org/show_bug.cgi?id=196887
+        <rdar://problem/49870627>
+
+        Reviewed by Timothy Hatcher.
+
+        Test: inspector/dom/event-listener-add-remove.html
+              inspector/dom/event-listener-inspected-node.html
+
+        * inspector/agents/InspectorDOMAgent.h:
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::getEventListenersForNode):
+        (WebCore::InspectorDOMAgent::setInspectedNode):
+        (WebCore::InspectorDOMAgent::didAddEventListener):
+        (WebCore::InspectorDOMAgent::willRemoveEventListener):
+
 2019-04-15  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp

@@ -953 +953 @@
         }
     }
+
+    if (m_inspectedNode == node)
+        m_suppressEventListenerChangedEvent = false;
 }
 
@@ -1399 +1402 @@
     }
 
+    m_inspectedNode = node;
+
     if (CommandLineAPIHost* commandLineAPIHost = static_cast<WebInjectedScriptManager&>(m_injectedScriptManager).commandLineAPIHost())
         commandLineAPIHost->addInspectedObject(std::make_unique<InspectableNode>(node));
+
+    m_suppressEventListenerChangedEvent = false;
 }
 
@@ -2389 +2396 @@
         return;
 
-    int nodeId = boundNodeId(&downcast<Node>(target));
+    auto& node = downcast<Node>(target);
+    if (!node.contains(m_inspectedNode.get()))
+        return;
+
+    int nodeId = boundNodeId(&node);
     if (!nodeId)
         return;
 
+    if (m_suppressEventListenerChangedEvent)
+        return;
+
+    m_suppressEventListenerChangedEvent = true;
+
     m_frontendDispatcher->didAddEventListener(nodeId);
 }
@@ -2400 +2416 @@
     if (!is<Node>(target))
         return;
+
     auto& node = downcast<Node>(target);
+    if (!node.contains(m_inspectedNode.get()))
+        return;
 
     int nodeId = boundNodeId(&node);
@@ -2420 +2439 @@
         return entry.value.matches(target, eventType, listener, capture);
     });
+
+    if (m_suppressEventListenerChangedEvent)
+        return;
+
+    m_suppressEventListenerChangedEvent = true;
 
     m_frontendDispatcher->willRemoveEventListener(nodeId);

trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h

@@ -252 +252 @@
     RefPtr<Node> m_nodeToFocus;
     RefPtr<Node> m_mousedOverNode;
+    RefPtr<Node> m_inspectedNode;
     std::unique_ptr<HighlightConfig> m_inspectModeHighlightConfig;
     std::unique_ptr<InspectorHistory> m_history;
@@ -257 +258 @@
     bool m_searchingForNode { false };
     bool m_suppressAttributeModifiedEvent { false };
+    bool m_suppressEventListenerChangedEvent { false };
     bool m_documentRequested { false };
 

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-04-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Elements: event listener change events should only be fired for the selected node and it's ancestors
+        https://bugs.webkit.org/show_bug.cgi?id=196887
+        <rdar://problem/49870627>
+
+        Reviewed by Timothy Hatcher.
+
+        * UserInterface/Models/DOMNode.js:
+        (WI.DOMNode.prototype.getEventListeners):
+
 2019-04-15  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Models/DOMNode.js

@@ -588 +588 @@
     getEventListeners(callback)
     {
+        console.assert(WI.domManager.inspectedNode === this);
         DOMAgent.getEventListenersForNode(this.id, callback);
     }