Skip to content

Commit

Permalink
Web Inspector: provide a way to enable/disable event listeners
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=177451

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

* inspector/protocol/DOM.json:
Add `setEventListenerDisabled` command that enables/disables a specific event listener
during event dispatch. When a disabled event listener is fired, the listener's callback will
not be called.

Source/WebCore:

Test: inspector/dom/setEventListenerDisabled.html

* dom/EventTarget.cpp:
(WebCore::EventTarget::fireEventListeners):
Add InspectorInstrumentation call to isEventListenerDisabled. If true, the event listener's
callback will not be called.

* inspector/InspectorDOMAgent.h:
* inspector/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::discardBindings):
(WebCore::InspectorDOMAgent::getEventListenersForNode):
(WebCore::InspectorDOMAgent::setEventListenerDisabled):
(WebCore::InspectorDOMAgent::buildObjectForEventListener):
(WebCore::InspectorDOMAgent::willRemoveEventListener):
(WebCore::InspectorDOMAgent::isEventListenerDisabled):
Introduce a mapping of `EventListener*` to `InspectorEventListener`, a struct for uniquely
identifying event listeners so they can be referenced from the frontend. We only add items
to this mapping when `getEventListenersForNode` is called, as that is when EventListener
data is sent to the frontend. This allows us to defer creating an Inspector "mirror" object
for each EventListener until it is needed. Items are removed whenever an event listener is
removed or when the document changes.

* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::isEventListenerDisabled):
* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::willRemoveEventListenerImpl):
(WebCore::InspectorInstrumentation::isEventListenerDisabledImpl):
Pass additional parameters to InspectorDOMAgent so it can determine if the event listener
actually exists. If not, don't dispatch an event to the frontend as nothing will change.

Source/WebInspectorUI:

* Localizations/en.lproj/localizedStrings.js:

* UserInterface/Controllers/DOMTreeManager.js:
(WI.DOMTreeManager.prototype.setEventListenerDisabled):

* UserInterface/Views/DOMNodeDetailsSidebarPanel.js:
(WI.DOMNodeDetailsSidebarPanel.prototype.attached):
(WI.DOMNodeDetailsSidebarPanel.prototype.detached):
(WI.DOMNodeDetailsSidebarPanel.prototype._eventListenersChanged):
(WI.DOMNodeDetailsSidebarPanel.prototype.addEventListeners): Deleted.
(WI.DOMNodeDetailsSidebarPanel.prototype.removeEventListeners): Deleted.
Listen for `WI.DOMNode.Event.EventListenersChanged` on all instances of WI.DOMNode, since we
will still want to refresh the event listeners section in the event that an event listener
is removed from a parent node.

* UserInterface/Views/EventListenerSectionGroup.js:
(WI.EventListenerSectionGroup):
(WI.EventListenerSectionGroup.prototype._eventText):
(WI.EventListenerSectionGroup.prototype._nodeTextOrLink):
(WI.EventListenerSectionGroup.prototype._createDisabledToggleElement):
(WI.EventListenerSectionGroup.prototype._createDisabledToggleElement.updateTitle):
* UserInterface/Views/EventListenerSectionGroup.css:
(.event-listener-section > .content input[type="checkbox"]):

* UserInterface/Views/DetailsSectionSimpleRow.js:
(WI.DetailsSectionSimpleRow.prototype.get label):
(WI.DetailsSectionSimpleRow.prototype.set label):

LayoutTests:

* inspector/dom/setEventListenerDisabled-expected.txt: Added.
* inspector/dom/setEventListenerDisabled.html: Added.


Canonical link: https://commits.webkit.org/194528@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223321 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dcrousso committed Oct 14, 2017
1 parent d614dd3 commit 1de3590
Show file tree
Hide file tree
Showing 18 changed files with 442 additions and 21 deletions.
10 changes: 10 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
2017-10-14 Devin Rousso <webkit@devinrousso.com>

Web Inspector: provide a way to enable/disable event listeners
https://bugs.webkit.org/show_bug.cgi?id=177451

Reviewed by Joseph Pecoraro.

* inspector/dom/setEventListenerDisabled-expected.txt: Added.
* inspector/dom/setEventListenerDisabled.html: Added.

2017-10-14 Per Arne Vollan <pvollan@apple.com>

Mark fast/frames/frame-unload-navigate-and-setTimeout-assert-fail.html as a flaky crash on Windows.
Expand Down
22 changes: 22 additions & 0 deletions LayoutTests/inspector/dom/setEventListenerDisabled-expected.txt
@@ -0,0 +1,22 @@
Testing DOMAgent.setEventListenerDisabled.


== Running test suite: DOM.setEventListenerDisabled
-- Running test case: DOM.setEventListenerDisabled.DisabledClickEvent
Click event listener is enabled.
Disabling event listener...
<body> clicked.
PASS: Click event listener did not fire.
Click event listener is disabled.

-- Running test case: DOM.setEventListenerDisabled.ReenabledClickEvent
Click event listener is disabled.
Enabling event listener...
<body> clicked.
PASS: Click event listener fired.
Click event listener is enabled.

-- Running test case: DOM.setEventListenerDisabled.Invalid
PASS: Should produce an error.
Error: No event listener for given identifier.

129 changes: 129 additions & 0 deletions LayoutTests/inspector/dom/setEventListenerDisabled.html
@@ -0,0 +1,129 @@
<!doctype html>
<html>
<head>
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
<script>
function clickBody() {
document.body.click();

TestPage.dispatchEventToFrontend("TestPageAfterClick");
}

function test() {
let clickEventListener = null;

let suite = InspectorTest.createAsyncSuite("DOM.setEventListenerDisabled");

function logListener() {
return DOMAgent.getEventListenersForNode(clickEventListener.nodeId).then(({listeners}) => {
InspectorTest.assert(listeners.length === 1, "There should only be one event listener.");
InspectorTest.assert(listeners[0].type === "click", `There event listener should be for "click".`);
InspectorTest.log("Click event listener is " + (listeners[0].disabled ? "disabled" : "enabled") + ".");
});
}

suite.addTestCase({
name: "DOM.setEventListenerDisabled.DisabledClickEvent",
test(resolve, reject) {
let listener = InspectorTest.singleFireEventListener("TestPageDocumentClicked", () => {
reject("Click event listener should not be called");
});

InspectorTest.singleFireEventListener("TestPageAfterClick", () => {
InspectorTest.pass("Click event listener did not fire.");

InspectorTest.removeEventListener(listener);

logListener().then(resolve, reject);
});

logListener().then(() => {
InspectorTest.log("Disabling event listener...");

const disabled = true;
DOMAgent.setEventListenerDisabled(clickEventListener.eventListenerId, disabled, (error) => {
if (error) {
reject(error);
return;
}

InspectorTest.evaluateInPage(`clickBody()`, () => {
InspectorTest.log("<body> clicked.");
});
});
});
}
});

suite.addTestCase({
name: "DOM.setEventListenerDisabled.ReenabledClickEvent",
test(resolve, reject) {
InspectorTest.singleFireEventListener("TestPageDocumentClicked", () => {
InspectorTest.pass("Click event listener fired.");

logListener().then(resolve, reject);
});

logListener().then(() => {
InspectorTest.log("Enabling event listener...");

const disabled = false;
DOMAgent.setEventListenerDisabled(clickEventListener.eventListenerId, disabled, (error) => {
if (error) {
reject(error);
return;
}

InspectorTest.evaluateInPage(`clickBody()`, () => {
InspectorTest.log("<body> clicked.");
});
});
});
}
});

suite.addTestCase({
name: "DOM.setEventListenerDisabled.Invalid",
description: "Invalid event listener identifiers should cause an error.",
test(resolve, reject) {
const invalidEventListenerId = 9999999;
const disabled = false;
DOMAgent.setEventListenerDisabled(invalidEventListenerId, disabled, (error) => {
InspectorTest.expectThat(error, "Should produce an error.");
InspectorTest.log("Error: " + error);
resolve();
});
}
});

WI.domTreeManager.requestDocument((documentNode) => {
DOMAgent.getEventListenersForNode(documentNode.id, (error, eventListeners) => {
if (error) {
InspectorTest.fail("Unable to retrieve event listeners.");
InspectorTest.completeTest();
return;
}

clickEventListener = eventListeners[0];
if (!clickEventListener || clickEventListener.type !== "click") {
InspectorTest.fail("Missing click event listener.");
InspectorTest.completeTest();
return;
}

suite.runTestCasesAndFinish();
});
});
}
</script>
</head>
<body onload="runTest()">
<p>Testing DOMAgent.setEventListenerDisabled.</p>

<script>
document.addEventListener("click", (event) => {
TestPage.dispatchEventToFrontend("TestPageDocumentClicked");
});
</script>
</body>
</html>
12 changes: 12 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,15 @@
2017-10-14 Devin Rousso <webkit@devinrousso.com>

Web Inspector: provide a way to enable/disable event listeners
https://bugs.webkit.org/show_bug.cgi?id=177451

Reviewed by Joseph Pecoraro.

* inspector/protocol/DOM.json:
Add `setEventListenerDisabled` command that enables/disables a specific event listener
during event dispatch. When a disabled event listener is fired, the listener's callback will
not be called.

2017-10-14 Yusuke Suzuki <utatane.tea@gmail.com>

Reland "Add Above/Below comparisons for UInt32 patterns"
Expand Down
17 changes: 16 additions & 1 deletion Source/JavaScriptCore/inspector/protocol/DOM.json
Expand Up @@ -13,6 +13,11 @@
"type": "integer",
"description": "Unique DOM node identifier used to reference a node that may not have been pushed to the front-end."
},
{
"id": "EventListenerId",
"type": "integer",
"description": "Unique event listener identifier."
},
{
"id": "PseudoType",
"type": "string",
Expand Down Expand Up @@ -74,6 +79,7 @@
"type": "object",
"description": "A structure holding event listener properties.",
"properties": [
{ "name": "eventListenerId", "$ref": "EventListenerId" },
{ "name": "type", "type": "string", "description": "<code>EventListener</code>'s type." },
{ "name": "useCapture", "type": "boolean", "description": "<code>EventListener</code>'s useCapture." },
{ "name": "isAttribute", "type": "boolean", "description": "<code>EventListener</code>'s isAttribute." },
Expand All @@ -83,7 +89,8 @@
{ "name": "sourceName", "type": "string", "optional": true, "description": "Source script URL." },
{ "name": "handler", "$ref": "Runtime.RemoteObject", "optional": true, "description": "Event handler function value." },
{ "name": "passive", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s passive." },
{ "name": "once", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s once." }
{ "name": "once", "type": "boolean", "optional": true, "description": "<code>EventListener</code>'s once." },
{ "name": "disabled", "type": "boolean", "optional": true }
]
},
{
Expand Down Expand Up @@ -258,6 +265,14 @@
{ "name": "listeners", "type": "array", "items": { "$ref": "EventListener"}, "description": "Array of relevant listeners." }
]
},
{
"name": "setEventListenerDisabled",
"description": "Enable/disable the given event listener. A disabled event listener will not fire.",
"parameters": [
{ "name": "eventListenerId", "$ref": "EventListenerId" },
{ "name": "disabled", "type": "boolean" }
]
},
{
"name": "getAccessibilityPropertiesForNode",
"description": "Returns a dictionary of accessibility properties for the node.",
Expand Down
37 changes: 37 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
2017-10-14 Devin Rousso <webkit@devinrousso.com>

Web Inspector: provide a way to enable/disable event listeners
https://bugs.webkit.org/show_bug.cgi?id=177451

Reviewed by Joseph Pecoraro.

Test: inspector/dom/setEventListenerDisabled.html

* dom/EventTarget.cpp:
(WebCore::EventTarget::fireEventListeners):
Add InspectorInstrumentation call to isEventListenerDisabled. If true, the event listener's
callback will not be called.

* inspector/InspectorDOMAgent.h:
* inspector/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::discardBindings):
(WebCore::InspectorDOMAgent::getEventListenersForNode):
(WebCore::InspectorDOMAgent::setEventListenerDisabled):
(WebCore::InspectorDOMAgent::buildObjectForEventListener):
(WebCore::InspectorDOMAgent::willRemoveEventListener):
(WebCore::InspectorDOMAgent::isEventListenerDisabled):
Introduce a mapping of `EventListener*` to `InspectorEventListener`, a struct for uniquely
identifying event listeners so they can be referenced from the frontend. We only add items
to this mapping when `getEventListenersForNode` is called, as that is when EventListener
data is sent to the frontend. This allows us to defer creating an Inspector "mirror" object
for each EventListener until it is needed. Items are removed whenever an event listener is
removed or when the document changes.

* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::isEventListenerDisabled):
* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::willRemoveEventListenerImpl):
(WebCore::InspectorInstrumentation::isEventListenerDisabledImpl):
Pass additional parameters to InspectorDOMAgent so it can determine if the event listener
actually exists. If not, don't dispatch an event to the frontend as nothing will change.

2017-10-14 Sam Weinig <sam@webkit.org>

Remove HashCountedSet's copyToVector functions
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/dom/EventTarget.cpp
Expand Up @@ -278,6 +278,9 @@ void EventTarget::fireEventListeners(Event& event, EventListenerVector listeners
if (event.eventPhase() == Event::BUBBLING_PHASE && registeredListener->useCapture())
continue;

if (InspectorInstrumentation::isEventListenerDisabled(*this, event.type(), registeredListener->callback(), registeredListener->useCapture()))
continue;

// If stopImmediatePropagation has been called, we just break out immediately, without
// handling any more events on this target.
if (event.immediatePropagationStopped())
Expand Down

0 comments on commit 1de3590

Please sign in to comment.