[webkit-reviews] review granted: [Bug 207968] Web Inspector: VoiceOver: TreeOutline does not correctly indicate selected item : [Attachment 391208] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 207968: Web Inspector: VoiceOver: TreeOutline does not correctly indicate
selected item
https://bugs.webkit.org/show_bug.cgi?id=207968

Attachment 391208: Patch

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




--- Comment #7 from Brian Burg <bburg at apple.com> ---
Comment on attachment 391208
  --> https://bugs.webkit.org/attachment.cgi?id=391208
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:8
> +	   Previously, the entire TreeOutline's DOM element had focus. With
this patch,

You didn't explain what the code changes do to affect focus, whether real focus
or that reported to VO. Is it .ariaSelected, .ariaExpanded, or .tabIndex?

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:64
> +.tree-outline.dom:not(.non-selectable):focus-within li.selected
.selection-area {

Interesting! Is this relevant to the VO fix, though?

> Source/WebInspectorUI/UserInterface/Views/TreeElement.js:-519
> -	   if (!omitFocus)

Any particular reason to reorder these if statements?

> Source/WebInspectorUI/UserInterface/Views/TreeElement.js:568
> +	       this._listItemNode.removeAttribute("tabIndex");

It bothers me that tabIndex is set on focus but cleared on deselect. Can it be
set on select more directly? Or can deselect() call an unfocus() method?