Summary: | Web Inspector: AX: Add additional contextual labels for Error/Warnings/Logs that will be spoken for screen readers | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Craig <jcraig> | ||||
Component: | Web Inspector | Assignee: | James Craig <jcraig> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, graouts, jcraig, joepeck, timothy, webkit-bug-importer | ||||
Priority: | P3 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 115976 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
James Craig
2013-06-26 17:24:20 PDT
Dependent on bug 115976 Created attachment 208968 [details]
patch
Comment on attachment 208968 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=208968&action=review Nice! > Source/WebInspectorUI/UserInterface/ConsoleCommandResult.js:49 > + if (!element.getAttribute("data-labelprefix")) Nit: This check could be hasAttribute instead of getAttribute. Comment on attachment 208968 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=208968&action=review > Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js:486 > + element.setAttribute("data-labelprefix", WebInspector.UIString("Tip: ")); We could even use elem.dataset for "data-x" attributes: element.dataset.labelprefix = WebInspector.UIString("Tip: "); However, I'm not sure if dataset is any clearer or more performant. So lets stick with your setAttribute calls. It makes it easy to search for "data-labelprefix" and find all the places we deal with it in code. (In reply to comment #4) > (From update of attachment 208968 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208968&action=review > > Nice! > > > Source/WebInspectorUI/UserInterface/ConsoleCommandResult.js:49 > > + if (!element.getAttribute("data-labelprefix")) > > Nit: This check could be hasAttribute instead of getAttribute. hasAttribute won't cover the empty string case, I believe. Without my shorthand version, I'd need to do this to cover both cases. if (!element.hasAttribute("data-labelprefix") || element.getAttribute("data-labelprefix") !== "") But I like the shorthand version better. (In reply to comment #5) > (From update of attachment 208968 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208968&action=review > > > Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js:486 > > + element.setAttribute("data-labelprefix", WebInspector.UIString("Tip: ")); > > We could even use elem.dataset for "data-x" attributes: > > element.dataset.labelprefix = WebInspector.UIString("Tip: "); > > However, I'm not sure if dataset is any clearer or more performant. So lets stick with your setAttribute calls. It makes it easy to search for "data-labelprefix" and find all the places we deal with it in code. Good idea. I could go either way on this, but it looks like you're leaning slightly toward searchability, so I'll leave it the way it is for now. Comment on attachment 208968 [details] patch Clearing flags on attachment: 208968 Committed r154310: <http://trac.webkit.org/changeset/154310> All reviewed patches have been landed. Closing bug. |