Closed
Bug 430758
Opened 15 years ago
Closed 15 years ago
GetAccessible() could be fooled by additional table ancestor
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: evan.yan, Assigned: evan.yan)
Details
(Keywords: access, regression)
Attachments
(1 file, 1 obsolete file)
3.36 KB,
patch
|
surkov
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
Given a testcase like this <table> <tr> <td style="display:block"> <table style="display:inline"> <tr><td>cell0</td></tr> </table> </td> <td>cell1</td> </tr> </table> GetAccessible() could be fooled by the outside table, so that create accessible for cell0 wrongly.
Attachment #317662 -
Flags: review?(surkov.alexander)
Comment 2•15 years ago
|
||
Evan, is tableContent checked for validity (not NULL) before you use it in this patch? I notice there's a check for tableContent being not NULL below, but are we sure that we don't introduce a potential crasher here?
Marco, it's inside the while loop 1503 while ((tableContent = tableContent->GetParent()) != nsnull) {
Comment 4•15 years ago
|
||
Evan, could you look why we introduced the check for tableCellFrame and why we didn't use table tag name check? Was there special reason for this? How will we deal with nested td elements? Can you attach mochitest for this?
Comment 5•15 years ago
|
||
We introduced that in bug 404881 before what we used "table" tag name
Comment 6•15 years ago
|
||
Btw, would be nice to put into our mochitests the testcase from bug 409009
Comment 7•15 years ago
|
||
I don't see special reason to check tableCellFrame though we need to check how we should deal with nested table cells.
Attachment #317662 -
Attachment is obsolete: true
Attachment #318360 -
Flags: review?(surkov.alexander)
Attachment #317662 -
Flags: review?(surkov.alexander)
Comment 9•15 years ago
|
||
Evan, could you add some tests for nested table cells as well like <table> <tr><td>hello<td>hello</td></td></tr> </table> how we should/do deal with this?
Assignee | ||
Comment 10•15 years ago
|
||
I think that is interpreted by layout module. From the view of dom, it has no difference from <table> <tr><td>hello</td><td>hello</td></tr> </table>
Comment 11•15 years ago
|
||
(In reply to comment #10) > I think that is interpreted by layout module. From the view of dom, it has no > difference from > > <table> > <tr><td>hello</td><td>hello</td></tr> > </table> > How it's interpreted by layout module? I just think we may broke nsIAccessibleTable methods in this case.
Assignee | ||
Comment 12•15 years ago
|
||
In domi, these two cases have the same dom tree. They should be the same thing to a11y. I also confirmed that we created the same accessible tree. Anyway, I can add it into mochitest if you insist.
Comment 13•15 years ago
|
||
No, I don't. If those nested td is processed on DOM level then it's fine.
Comment 14•15 years ago
|
||
one more thing: how will we deal with this: <table> <tr><td> <div style="display: table"> <tr> <td>hee</td> </tr> </div> </td></tr> </table>
Assignee | ||
Comment 15•15 years ago
|
||
the dom tree is like table tbody tr td div tr td text ("hee") so we have accessible tree (on Solaris) table table cell table (div) table cell ("hee")
Comment 16•15 years ago
|
||
Comment on attachment 318360 [details] [diff] [review] add mochitest r=me then
Attachment #318360 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 17•15 years ago
|
||
Comment on attachment 318360 [details] [diff] [review] add mochitest it fixes a regression, and with low risk.
Attachment #318360 -
Flags: approval1.9?
Comment 18•15 years ago
|
||
Comment on attachment 318360 [details] [diff] [review] add mochitest a1.9=beltzner
Attachment #318360 -
Flags: approval1.9? → approval1.9+
Comment 19•15 years ago
|
||
Checking in accessible/src/base/nsAccessibilityService.cpp; /cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v <-- nsAccessibilityService.cpp new revision: 1.280; previous revision: 1.279 done Checking in accessible/tests/mochitest/test_nsIAccessibleTable_4.html; /cvsroot/mozilla/accessible/tests/mochitest/test_nsIAccessibleTable_4.html,v <-- test_nsIAccessibleTable_4.html new revision: 1.2; previous revision: 1.1 done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•