<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - AX: VoiceOver unable to access content in malformed trees"
   href="https://bugs.webkit.org/show_bug.cgi?id=147295#c13">Comment # 13</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - AX: VoiceOver unable to access content in malformed trees"
   href="https://bugs.webkit.org/show_bug.cgi?id=147295">bug 147295</a>
              from <span class="vcard"><a class="email" href="mailto:cfleizach&#64;apple.com" title="chris fleizach &lt;cfleizach&#64;apple.com&gt;"> <span class="fn">chris fleizach</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=257712&amp;action=diff" name="attach_257712" title="patch">attachment 257712</a> <a href="attachment.cgi?id=257712&amp;action=edit" title="patch">[details]</a></span>
patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=257712&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=257712&amp;action=review</a>

thanks

<span class="quote">&gt; Source/WebCore/ChangeLog:9
&gt; +        VoiceOver is skipping the content of the malformed trees. Fixed it by ignoring</span >

VoiceOver is skipping the content of malformed trees. Fixed it by ignoring the tree/treeitem attribute roles for those malformed trees.

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2505
&gt; +    if (m_ariaRole != UnknownRole &amp;&amp; !shouldIgnoreAriaRole)</span >

put this into the if statement for brevity
shouldIgnoreAttributeRole();

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityRenderObject.h:213
&gt; +    virtual bool shouldIgnoreAttributeRole() const { return false; }</span >

i would put this on AccessibilityObject

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityTree.cpp:75
&gt; +    std::queue&lt;Node*&gt; nodeQueue;</span >

lets use Dequeue here

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityTree.cpp:88
&gt; +        if (is&lt;Element&gt;(*child)) {</span >

i think we can cut down a lot of code using nodeHasRole(node, &quot;tree item&quot;) || nodeHas(&quot;group&quot;)

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityTree.cpp:99
&gt; +            for (Node* groupChild = child-&gt;firstChild(); groupChild; groupChild = groupChild-&gt;nextSibling())</span >

i think we can use auto in more places here

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityTree.h:46
&gt; +    bool isTreeValid();</span >

i think this can be const

<span class="quote">&gt; Source/WebCore/accessibility/AccessibilityTreeItem.cpp:60
&gt; +    AccessibilityObject* axObj = parentObject();</span >

i think we can write this as one for loop line

AXObject *ax= nullptr;
for ( = parent; parent &amp;&amp; !parent-&gt;IsTreE(); parent = next parent) {}

m_isTreeValid = parent;

<span class="quote">&gt; LayoutTests/platform/mac/accessibility/malformed-tree.html:38
&gt; +    shouldBe(&quot;tree.role&quot;, &quot;\&quot;AXRole: AXOutline\&quot;&quot;);</span >

&quot;\&quot;AXRole: AXOutline\&quot;&quot; =&gt;

&quot;'AXRole: AXOutline'&quot;</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>