Bug 193226 - Web Inspector: Audit: provide a way to determine whether a give node has event listeners
Summary: Web Inspector: Audit: provide a way to determine whether a give node has even...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: WebInspectorAuditTab 193149
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-07 19:44 PST by Devin Rousso
Modified: 2019-01-15 08:31 PST (History)
6 users (show)

See Also:


Attachments
Patch (7.21 KB, patch)
2019-01-07 19:46 PST, Devin Rousso
hi: commit-queue-
Details | Formatted Diff | Diff
Patch (12.45 KB, patch)
2019-01-10 16:37 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (11.09 KB, patch)
2019-01-15 01:03 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (11.25 KB, patch)
2019-01-15 01:09 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.52 MB, application/zip)
2019-01-15 03:08 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-01-07 19:44:28 PST
This is similar to the Console's `getEventListeners` or `DOM.getEventListenersForNode`, except that it returns true/false.
Comment 1 Devin Rousso 2019-01-07 19:44:41 PST
<rdar://problem/46800005>
Comment 2 Devin Rousso 2019-01-07 19:46:21 PST
Created attachment 358566 [details]
Patch
Comment 3 Devin Rousso 2019-01-10 16:37:37 PST
Created attachment 358849 [details]
Patch

Throw an error when called outside of a test
Comment 4 Joseph Pecoraro 2019-01-14 14:02:28 PST
Comment on attachment 358849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358849&action=review

r=me

> LayoutTests/inspector/audit/run-dom.html:13
> +    function evaluateStringForTest(func, target, args) {
> +        return `DOM.${func}(document.querySelector("#${target}")${args ? ", " + JSON.stringify(args) : ""})`;
> +    }

This kind of code generation makes the test compact but hurts readability and comprehension. If this test ever fails someone has to wade through this to figure out whats going on, when instead there could have been a straightforward test with a little duplication but much clearer execution. You don't have to change the test, but this is something to consider in the future.

> LayoutTests/inspector/audit/run-dom.html:53
> +            let functions = new Map;
> +            for (let test of tests)
> +                functions.set(test.func, test);

So this only performs the last `hasEventListeners` test.

> Source/WebCore/inspector/InspectorAuditDOMUtilities.cpp:38
> +#define ERROR_IF_NO_ACTIVE_AUDIT() if (!m_auditAgent.hasActiveAudit()) return Exception { NotAllowedError, "Unable to run outside of an Inspector Audit"_s };

Style: Make this multiple lines to improve readability.
Comment 5 Devin Rousso 2019-01-15 01:00:31 PST
Comment on attachment 358849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358849&action=review

>> LayoutTests/inspector/audit/run-dom.html:53
>> +                functions.set(test.func, test);
> 
> So this only performs the last `hasEventListeners` test.

The point of this isn't really to test every case from above, more-so to just check that calling the function at all outside of a test will throw an error.  The arguments are irrelevant.
Comment 6 Devin Rousso 2019-01-15 01:03:33 PST
Created attachment 359145 [details]
Patch
Comment 7 Devin Rousso 2019-01-15 01:09:55 PST
Created attachment 359146 [details]
Patch

AUDIT => WebInspectorAudit
Comment 8 EWS Watchlist 2019-01-15 03:08:02 PST Comment hidden (obsolete)
Comment 9 EWS Watchlist 2019-01-15 03:08:04 PST Comment hidden (obsolete)
Comment 10 WebKit Commit Bot 2019-01-15 08:31:11 PST
Comment on attachment 359146 [details]
Patch

Clearing flags on attachment: 359146

Committed r239987: <https://trac.webkit.org/changeset/239987>
Comment 11 WebKit Commit Bot 2019-01-15 08:31:12 PST
All reviewed patches have been landed.  Closing bug.