WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
62335
VoiceOver cannot navigate the itunes album view table
https://bugs.webkit.org/show_bug.cgi?id=62335
Summary
VoiceOver cannot navigate the itunes album view table
chris fleizach
Reported
2011-06-08 15:47:54 PDT
On iOS, VoiceOver cannot navigate the iTunes album view table. This is a regression from
https://bugs.webkit.org/show_bug.cgi?id=57463
Attachments
patch
(8.93 KB, patch)
2011-06-08 15:53 PDT
,
chris fleizach
ddkilzer
: review+
ddkilzer
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
chris fleizach
Comment 1
2011-06-08 15:49:18 PDT
The problem is how rows of an ARIA grid are determined. The mentioned bug was strangely and wrongly only adding children 1 level deep in the hierarchy. This means that it doesn't look any farther for table rows.
chris fleizach
Comment 2
2011-06-08 15:53:15 PDT
Created
attachment 96501
[details]
patch
David Kilzer (:ddkilzer)
Comment 3
2011-06-14 10:24:34 PDT
Comment on
attachment 96501
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=96501&action=review
r=me with the unsigned => size_t changes.
> Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:119 > + unsigned length = children.size();
This should be size_t instead of unsigned.
> Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:120 > + for (unsigned i = 0; i < length; ++i)
unsigned => size_t
chris fleizach
Comment 4
2011-06-14 11:20:49 PDT
http://trac.webkit.org/changeset/88830
chris fleizach
Comment 5
2011-06-14 12:55:16 PDT
fix the leopard test
http://trac.webkit.org/changeset/88843
chris fleizach
Comment 6
2011-06-14 12:55:58 PDT
I see a GTK test is failing as well. May need Mario's help to fix that one AXRole: table AXRole: table AXRole: table cell +AXRole: unknown AXRole: table cell -AXRole: table cell -AXRole: table cell +AXRole: unknown Test passed
chris fleizach
Comment 7
2011-06-14 12:56:10 PDT
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r88840%20(23362)/results.html
chris fleizach
Comment 8
2011-06-14 17:19:36 PDT
rdar://9600922
. Also affects the desktop
Ryosuke Niwa
Comment 9
2011-06-14 18:03:48 PDT
It seems like the test is still faling on Leopard.
http://build.webkit.org/results/Leopard%20Intel%20Release%20(Tests)/r88877%20(33189)/results.html
Could you look into it?
chris fleizach
Comment 10
2011-06-14 18:07:35 PDT
(In reply to
comment #9
)
> It seems like the test is still faling on Leopard. >
http://build.webkit.org/results/Leopard%20Intel%20Release%20(Tests)/r88877%20(33189)/results.html
> > Could you look into it?
looking into why my skipped list change didn't work
chris fleizach
Comment 11
2011-06-14 18:11:34 PDT
Fixed for real
http://trac.webkit.org/changeset/88887
Mario Sanchez Prada
Comment 12
2011-06-15 04:33:48 PDT
(In reply to
comment #6
)
> I see a GTK test is failing as well. May need Mario's help to fix that one > > AXRole: table > AXRole: table > AXRole: table cell > +AXRole: unknown > AXRole: table cell > -AXRole: table cell > -AXRole: table cell > +AXRole: unknown > Test passed
It seems the problem in GTK with this patch is that the addChild() method expects a row object with the cells as its children, and doesn't care initially whether that row object is ignoring a11y or not: it will just consider that later on, when deciding whether to add the row object itself to the hierarchy, or just the raw list of children: bool AccessibilityARIAGrid::addChild(AccessibilityObject* child, HashSet<AccessibilityObject*>& appendedRows, unsigned& columnCount) { if (!child || !child->isTableRow() || child->ariaRoleAttribute() != RowRole) return false; [...] // Try adding the row if it's not ignoring accessibility, // otherwise add its children (the cells) as the grid's children. if (!row->accessibilityIsIgnored()) m_children.append(row); else m_children.append(row->children()); appendedRows.add(row); return true; } Thus, when we do the following in AccessibilityARIAGrid::addChildren()... // The children of this non-row will contain all non-ignored elements (recursing to find them). // This allows the table to dive arbitrarily deep to find the rows. AccessibilityChildrenVector children = child->children(); unsigned length = children.size(); for (unsigned i = 0; i < length; ++i) addChild(children[i].get(), appendedRows, columnCount); } ...nothing is being added to the hierarchy in GTK, because navigating through this AccessibilityChildrenVector, as Chris explained in the comment, will mean navigating through non-ignored elements only, which in GTK means just the cells, as rows won't be exposed in that platform (yest, weird, but at the moment it's like that, will change in the future, though [1][2]). So, the problem I see with this patch, from GTK's selfish point of view, is that it assumes that rows are not being ignored, which is right for the Mac, but not for GTK at this point. My proposal to keep this regression fixed while still keep all the parties happy, is just to combine the code before and after this patch with #if - #endif regions, doing something like this: #if PLATFORM(GTK) // Do not navigate children through the Accessibility children vector to let addChild() check // the result of accessibilityIsIgnored() and make the proper decision. AccessibilityObject* grandChild = 0; for (grandChild = child->firstChild(); grandChild; grandChild = grandChild->nextSibling()) addChild(grandChild, appendedRows, columnCount); #else // The children of this non-row will contain all non-ignored elements (recursing to find them). // This allows the table to dive arbitrarily deep to find the rows. AccessibilityChildrenVector children = child->children(); size_t length = children.size(); for (size_t i = 0; i < length; ++i) addChild(children[i].get(), appendedRows, columnCount); #endif Why I propose this ugly thing? Well, because after all this hack is needed because of a very specific requirement in GTK (not exposing rows) that is needed at the moment, but that was already decided it will change in the future (see ATK hackfest agenda and conclusions in [1] and [2]), so when that happens it will be just a matter of removing this #if PLATFORM(GTK) - #endif region and just leaving the generic one Chris wrote, and that should work for all the parties. As this seems to be a too-specific thing for GTK, I will file a new bug and propose this patch there, keeping the test skipped while it's not fixed. [1]
https://live.gnome.org/Hackfests/ATK2011/Agenda
[2]
https://live.gnome.org/Hackfests/ATK2011/Agenda/table
Mario Sanchez Prada
Comment 13
2011-06-15 04:47:44 PDT
(In reply to
comment #12
)
> [...] > As this seems to be a too-specific thing for GTK, I will file a new bug and propose this patch there, keeping the test skipped while it's not fixed.
Filed
bug 62718
for tracking down this issue. Test skipped in the meanwhile:
http://trac.webkit.org/changeset/88919
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug