[webkit-reviews] review granted: [Bug 193228] Web Inspector: Audit: provide a way to query for all nodes with a given computed Accessibility role : [Attachment 358850] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 14 14:06:57 PST 2019


Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 193228: Web Inspector: Audit: provide a way to query for all nodes with a
given computed Accessibility role
https://bugs.webkit.org/show_bug.cgi?id=193228

Attachment 358850: Patch

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




--- Comment #4 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 358850
  --> https://bugs.webkit.org/attachment.cgi?id=358850
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.


More information about the webkit-reviews mailing list