Bug 193228 – Web Inspector: Audit: provide a way to query for all nodes with a given computed Accessibility role

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

This would be functionally similar to `node.querySelectorAll(*[aria-role="..."])`, but would instead compare to the computed role (e.g. not the markup value).

[Devin Rousso]

<rdar://problem/46787787>

[Devin Rousso]

Created attachment 358572 [details]
Patch

[Devin Rousso]

Created attachment 358850 [details]
Patch

Throw an error when called outside of a test

[Joseph Pecoraro]

Comment on attachment 358850 [details]
Patch

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

r=me

> LayoutTests/inspector/audit/run-accessibility-expected.txt:22
> +-- Running test case: Audit.run.Accessibility.getElementsByComputedRole.button
> +Audit setup...
> +Audit run `AUDIT.Accessibility.getElementsByComputedRole("button")`...
> +Result: ["#button"]
> +Audit teardown...

Lets have a test that returns multiple elements along side this one which just returns one.

> LayoutTests/inspector/audit/run-accessibility.html:29
> +    function evaluateStringForTest(func, target, role) {
> +        if (target) {
> +            if (role)
> +                return `Accessibility.${func}("${role}", document.querySelector("#${target}"))`;
> +            return `Accessibility.${func}(document.querySelector("#${target}"))`;
> +        }
> +
> +        if (role)
> +            return `Accessibility.${func}("${role}")`;
> +    }

Having read a few of these now, I think having `AUDIT.Accessibility` here would be better for searchability as well instead of putting the AUDIT part later. Up to you.

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.cpp:42
> +#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.

[Devin Rousso]

Created attachment 359148 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 359148 [details]
Patch

Clearing flags on attachment: 359148

Committed r239986: <https://trac.webkit.org/changeset/239986>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The new test inspector/audit/run-accessibility.html

added in https://trac.webkit.org/changeset/239986/webkit

is a flakey failure on Mac.

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Faudit%2Frun-accessibility.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/inspector/audit/run-accessibility-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/inspector/audit/run-accessibility-actual.txt
@@ -18,7 +18,7 @@
 -- Running test case: Audit.run.Accessibility.getElementsByComputedRole.tree
 Audit setup...
 Audit run `WebInspectorAudit.Accessibility.getElementsByComputedRole("tree")`...
-Result: ["#parent"]
+Result: []
 Audit teardown...
 
 -- Running test case: Audit.run.Accessibility.getElementsByComputedRole.tree.parent
@@ -30,7 +30,7 @@
 -- Running test case: Audit.run.Accessibility.getElementsByComputedRole.button
 Audit setup...
 Audit run `WebInspectorAudit.Accessibility.getElementsByComputedRole("button")`...
-Result: ["#button","#link"]
+Result: []
 Audit teardown...
 
 -- Running test case: Audit.run.Accessibility.getElementsByComputedRole.button.parent

[Devin Rousso]

(In reply to Truitt Savell from comment #8)
> The new test inspector/audit/run-accessibility.html
> 
> added in https://trac.webkit.org/changeset/239986/webkit
> 
> is a flakey failure on Mac.
This test only seems to fail on my machine if you run it in concert with other tests (or repeated iterations).  Running it by itself has always passed, but it doesn't seem to work with more than one run.

$ run-webkit-tests --no-build --no-sample --no-retry-failures --time-out-ms=5000 --force --verbose --iterations=100 --exit-after-n-failures=1 inspector/audit/run-accessibility.html

That seems to consistently fail on the 2nd or 3rd iteration.

Based on what I've been able to figure out so far, it seems that we aren't creating a render tree for the new document, meaning that we early-return for any accessibility related calls.

[Devin Rousso]

(In reply to Devin Rousso from comment #9)
> (In reply to Truitt Savell from comment #8)
> > The new test inspector/audit/run-accessibility.html
> > 
> > added in https://trac.webkit.org/changeset/239986/webkit
> > 
> > is a flakey failure on Mac.
> This test only seems to fail on my machine if you run it in concert with
> other tests (or repeated iterations).  Running it by itself has always
> passed, but it doesn't seem to work with more than one run.
> 
> $ run-webkit-tests --no-build --no-sample --no-retry-failures
> --time-out-ms=5000 --force --verbose --iterations=100
> --exit-after-n-failures=1 inspector/audit/run-accessibility.html
> 
> That seems to consistently fail on the 2nd or 3rd iteration.
> 
> Based on what I've been able to figure out so far, it seems that we aren't
> creating a render tree for the new document, meaning that we early-return
> for any accessibility related calls.
It looks like the problem was "higher up" than the Document.  The implementation of `getElementsByComputedRole` used `[CallWith=Document]` so that it could work without hassle inside any environment (rather than relying on the inspected page's main frame's document).  The issue was that the backing WebCore `InspectorAuditAccessibilityObject` was being reused across test runs, meaning that even though the page changed, the underlying object didn't.  My guess is that the reference to the JavaScript value was somehow being cached between these runs, and when we `toJS` the `InspectorAuditAccessibilityObject`, we get the old page's JavaScript value instead of the new one, which no longer has a render tree, meaning it wouldn't generate any accessibility objects.

<https://webkit.org/b/193594>