[Webkit-unassigned] [Bug 201080] Web Inspector: AXI: Audit: obtuse error strings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=201080

Devin Rousso <drousso at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #408044|review?                     |review+
              Flags|                            |

--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 408044
  --> https://bugs.webkit.org/attachment.cgi?id=408044
Patch

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

r=me, with some tweaks :)

> Source/WebInspectorUI/UserInterface/Controllers/AuditManager.js:1016
>                  ], {description: WI.UIString("These are all of the different types of data that can be returned with the test result.")}),

let's be consistent with all of the group descriptions

    "These are example tests that demonstrate ..."

> Source/WebInspectorUI/UserInterface/Controllers/AuditManager.js:1019
> +                        new WI.AuditTestCase("getElementsByComputedRole", removeWhitespace(getElementsByComputedRole), {description: WI.UIString("This is an example test that uses the getElementsByComputedRole method to find elements with a computed role of \u0022link\u0022."), supports: 1}),

I think we should be explicit about what we mean by `getElementsByComputedRole` in that it's really `WebInspectorAudit.Accessibility.getElementsByComputedRole`.  This way, developers who look at this will know exactly what it's doing and how to replicate it at a glance.

    WI.UIString("This is an example test that uses WebInspectorAudit.Accessibility.getElementsByComputedRole to find elements with a computed role of \u0022%s\u0022.").format(WI.unlocalizedString("link")

ditto for the other functions below

> Source/WebInspectorUI/UserInterface/Controllers/AuditManager.js:1029
>                      ], {description: WI.UIString("These tests demonstrate how to use %s to get information about the accessibility tree.").format(WI.unlocalizedString("WebInspectorAudit.Accessibility")), supports: 1}),

ditto (:1016)

> Source/WebInspectorUI/UserInterface/Controllers/AuditManager.js:1033
>                      ], {description: WI.UIString("These tests demonstrate how to use %s to get information about DOM nodes.").format(WI.unlocalizedString("WebInspectorAudit.DOM")), supports: 1}),

ditto (:1016)

> Source/WebInspectorUI/UserInterface/Controllers/AuditManager.js:1039
> +                new WI.AuditTestCase("unsupported", removeWhitespace(unsupported), {description: WI.UIString("This is an example test that shows the %s behavior when using an unsupported method or value. This test will not run because it is unsupported.").format(WI.unlocalizedString("WebInspectorAudit")), supports: Infinity}),

This description isn't accurate, as it has nothing to do with `WebInspectorAudit`.  It'd be more accurate to say:

    "This is an example of a test that will not run because it is unsupported."

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200905/ad352fb2/attachment.htm>