<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:cfleizach@apple.com" title="chris fleizach <cfleizach@apple.com>"> <span class="fn">chris fleizach</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: iframe within table cell is inaccessible to VoiceOver"
href="https://bugs.webkit.org/show_bug.cgi?id=147001">bug 147001</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #256941 Flags</td>
<td>commit-queue?
</td>
<td>review-, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: iframe within table cell is inaccessible to VoiceOver"
href="https://bugs.webkit.org/show_bug.cgi?id=147001#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: iframe within table cell is inaccessible to VoiceOver"
href="https://bugs.webkit.org/show_bug.cgi?id=147001">bug 147001</a>
from <span class="vcard"><a class="email" href="mailto:cfleizach@apple.com" title="chris fleizach <cfleizach@apple.com>"> <span class="fn">chris fleizach</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=256941&action=diff" name="attach_256941" title="patch">attachment 256941</a> <a href="attachment.cgi?id=256941&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=256941&action=review">https://bugs.webkit.org/attachment.cgi?id=256941&action=review</a>
make sure you add r? if you want a review. you should always do that before adding cq?
<span class="quote">> Source/WebCore/ChangeLog:9
> + Update each table cell's role once the table adds children.</span >
change this comment to one that explains the problem and explains the solution, succinctly.
"When a table cell is created before its parent table determines if it should be ignored or not, the table cell may cache the wrong role.
Fix that allowing each table cell to update its role after the table makes this determination."
<span class="quote">> Source/WebCore/accessibility/AccessibilityTable.cpp:413
> + // Sometimes the cell gets wrong role that it thinks the parent table is</span >
--> change to :
"Sometimes the cell gets the wrong role initially because it is created before the parent determines whether it is an accessibility table.
Iterate all the cells and allow them to update their roles now that the table knows its status."
<span class="quote">> Source/WebCore/accessibility/AccessibilityTable.cpp:417
> + if (isExposableThroughAccessibility()) {</span >
we already check this at the top of this method, so this is not necessary to call again
<span class="quote">> Source/WebCore/accessibility/AccessibilityTable.cpp:418
> + for (const auto& row : children()) {</span >
you can just integrate through m_rows and then you don't have to check row->roleValue()
<span class="quote">> Source/WebCore/accessibility/AccessibilityTable.cpp:422
> + AccessibilityObject* cellObj = cell.get();</span >
doesn't seem necessary to create this local variable
<span class="quote">> LayoutTests/ChangeLog:9
> + Test with iframe within table cell, expect to get the correct cell role as AXCell.</span >
Generally no comment is needed for a layout test that comes with a bug fix, so i would remove this line
<span class="quote">> LayoutTests/accessibility/iframe-within-cell.html:14
> + document.body.focus();</span >
don't use tabs. only use 4 spaces
<span class="quote">> LayoutTests/accessibility/iframe-within-cell.html:17
> + result.innerText += "Role for iframe cell is: " + frameRole + " .\n";</span >
you should use either
debug(message)
or
shouldBe("frame.role", "'AXRole: AXCell'");
instead of appending text to the result div manually</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>