[webkit-reviews] review granted: [Bug 193225] Web Inspector: Audit: provide a way to get related Accessibility nodes for a given node : [Attachment 358858] Patch

[bugzilla-daemon at webkit.org]

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

Attachment 358858: Patch

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




--- Comment #6 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 358858
  --> https://bugs.webkit.org/attachment.cgi?id=358858
Patch

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

r=me, earlier patch comments applying.

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.cpp:109
> +	       if (controlledElement)

Nit: Is this if check necessary, would elementsFromAttribute return a null
element?

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.cpp:131
> +	       if (flowedElement)

Nit: Is this if check necessary, would elementsFromAttribute return a null
element?

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.cpp:148
> +    }
> +    return nullptr;

Style: Sometimes you don't have a newline before the final return and sometimes
you do. Would be nice to be consistent. I'd vote for always having a newline
after a closing brace before a final return.

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.cpp:164
> +		   if (ownedElement)

Nit: Is this if check necessary, would elementsFromAttribute return a null
element?