AX: cellForColumnAndRow fails for tables with hidden table cells
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2013 07:13:46 +0000 (07:13 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2013 07:13:46 +0000 (07:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110050

Reviewed by Tim Horton.

Source/WebCore:

If a table had hidden cells, then accessibility code was being confused in a few ways.
1) The cellForColumnAndRow method would return the wrong information since that was
   using the RenderTableSection to retrieve a cell, which did not have the same data as the AXTable
2) The way we were adding children made it impossible to determine column and row range because we
   would skip rows that had hidden children
3) AccessibilityARIAGrid and AccessibilityTable were using different methods for cellForColumnAndRow

The fix does a few things to make things right:
1) Always add an accessibility row, even if there are no visible cells in that row.
2) Have one method for AXTable and AXARIAGrid for cellForColumnAndRow.
3) Change cellForColumnAndRow to query the accessibility children rather than the RenderTableSection in determining the row, col info.
4) cellForColumnAndRow should use unsigned values instead of int values.

Test: accessibility/table-with-hidden-head-section.html

* accessibility/AccessibilityARIAGrid.cpp:
(WebCore):
* accessibility/AccessibilityARIAGrid.h:
(AccessibilityARIAGrid):
* accessibility/AccessibilityARIAGridCell.cpp:
(WebCore::AccessibilityARIAGridCell::rowIndexRange):
(WebCore::AccessibilityARIAGridCell::columnIndexRange):
* accessibility/AccessibilityARIAGridCell.h:
(AccessibilityARIAGridCell):
* accessibility/AccessibilityTable.cpp:
(WebCore::AccessibilityTable::addChildren):
(WebCore::AccessibilityTable::cellForColumnAndRow):
* accessibility/AccessibilityTable.h:
(WebCore):
(AccessibilityTable):
* accessibility/AccessibilityTableCell.cpp:
(WebCore::AccessibilityTableCell::rowIndexRange):
(WebCore::AccessibilityTableCell::columnIndexRange):
* accessibility/AccessibilityTableCell.h:
(AccessibilityTableCell):
* accessibility/atk/WebKitAccessibleInterfaceTable.cpp:
(webkitAccessibleTableGetColumnAtIndex):
(webkitAccessibleTableGetRowAtIndex):
(webkitAccessibleTableGetColumnExtentAt):
(webkitAccessibleTableGetRowExtentAt):
(webkitAccessibleTableGetColumnHeader):
(webkitAccessibleTableGetRowHeader):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
* rendering/RenderTableSection.h:
(RenderTableSection):
(WebCore::RenderTableSection::rowRendererAt):

Source/WebKit/chromium:

* src/WebAccessibilityObject.cpp:
(WebKit::WebAccessibilityObject::cellColumnIndex):
(WebKit::WebAccessibilityObject::cellColumnSpan):
(WebKit::WebAccessibilityObject::cellRowIndex):
(WebKit::WebAccessibilityObject::cellRowSpan):

LayoutTests:

* accessibility/table-with-hidden-head-section-expected.txt: Added.
* accessibility/table-with-hidden-head-section.html: Added.
* platform/chromium/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@144727 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
LayoutTests/ChangeLog
LayoutTests/accessibility/table-with-hidden-head-section-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/table-with-hidden-head-section.html [new file with mode: 0644]
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityARIAGrid.cpp
Source/WebCore/accessibility/AccessibilityARIAGrid.h
Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp
Source/WebCore/accessibility/AccessibilityARIAGridCell.h
Source/WebCore/accessibility/AccessibilityTable.cpp
Source/WebCore/accessibility/AccessibilityTable.h
Source/WebCore/accessibility/AccessibilityTableCell.cpp
Source/WebCore/accessibility/AccessibilityTableCell.h
Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTable.cpp
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Source/WebCore/rendering/RenderTableSection.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebAccessibilityObject.cpp

index 25411a6..f8a3235 100644 (file)
@@ -1,3 +1,14 @@
+2013-03-04  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: cellForColumnAndRow fails for tables with hidden table cells
+        https://bugs.webkit.org/show_bug.cgi?id=110050
+
+        Reviewed by Tim Horton.
+
+        * accessibility/table-with-hidden-head-section-expected.txt: Added.
+        * accessibility/table-with-hidden-head-section.html: Added.
+        * platform/chromium/TestExpectations:
+
 2013-03-04  Arpita Bahuguna  <a.bah@samsung.com>
 
         getAttribute does not behave correctly for mixed-case attributes on HTML elements
diff --git a/LayoutTests/accessibility/table-with-hidden-head-section-expected.txt b/LayoutTests/accessibility/table-with-hidden-head-section-expected.txt
new file mode 100644 (file)
index 0000000..578e29d
--- /dev/null
@@ -0,0 +1,30 @@
+This tests that cellForRowAndColumn work as expected when cells are hidden in sections and rows.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+Table1 has a hidden first row. Verify accessing cells works as expected.
+PASS table1.rowCount is 2
+PASS !table1cell1 || !table1cell1.isValid is true
+PASS table1cell2.isEqual(accessibilityController.accessibleElementById('table1cell')) is true
+PASS table1cell2.rowIndexRange() is '{1, 1}'
+PASS table1cell2.columnIndexRange() is '{0, 1}'
+
+Table2 has a hidden first section. Verify accessing cells works as expected.
+PASS table2.rowCount is 2
+PASS !table2cell1 || !table2cell1.isValid is true
+PASS table2cell2.isEqual(accessibilityController.accessibleElementById('table2cell')) is true
+PASS table2cell2.rowIndexRange() is '{1, 1}'
+PASS table2cell2.columnIndexRange() is '{0, 1}'
+
+Table3 only has a footer section.
+PASS table3.rowCount is 3
+PASS !table3cell1 || !table3cell1.isValid is true
+PASS table3cell2.isEqual(accessibilityController.accessibleElementById('table3cell')) is true
+PASS table3cell2.rowIndexRange() is '{2, 1}'
+PASS table3cell2.columnIndexRange() is '{0, 1}'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/table-with-hidden-head-section.html b/LayoutTests/accessibility/table-with-hidden-head-section.html
new file mode 100644 (file)
index 0000000..6bff92d
--- /dev/null
@@ -0,0 +1,77 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+
+<div id="content">
+
+<!-- First row is hidden -->
+<table id="table1" border=1>
+  <tr><th hidden>header 1</th><th hidden>header 2</th></tr>
+  <tr><td id="table1cell">foo</td><td>bar</td></tr>
+</table>
+
+<!-- First section is hidden -->
+<table id="table2" border=1>
+  <thead><th hidden>header 1</th><th hidden>header 2</th></thead>
+  <tr><td id="table2cell">foo</td><td>bar</td></tr>
+</table>
+
+<!-- First and middle section is hidden -->
+<table id="table3" border=1>
+  <thead><tr><th hidden>header 1</th><th hidden>header 2</th></tr></thead>
+  <tr><td hidden>foo</td><td hidden>bar</td></tr>
+  <tfoot><tr><td id="table3cell">foot</td><td>foot</td></tr></tfoot>
+</table>
+
+</div>
+
+<script>
+
+    description("This tests that cellForRowAndColumn work as expected when cells are hidden in sections and rows.");
+
+    if (window.accessibilityController) {
+
+        debug("\nTable1 has a hidden first row. Verify accessing cells works as expected.");
+        var table1 = accessibilityController.accessibleElementById("table1");
+        shouldBe("table1.rowCount", "2");
+        var table1cell1 = table1.cellForColumnAndRow(0, 0);
+        shouldBeTrue("!table1cell1 || !table1cell1.isValid");
+        var table1cell2 = table1.cellForColumnAndRow(0, 1);
+        shouldBeTrue("table1cell2.isEqual(accessibilityController.accessibleElementById('table1cell'))");
+        shouldBe("table1cell2.rowIndexRange()", "'{1, 1}'");
+        shouldBe("table1cell2.columnIndexRange()", "'{0, 1}'");
+
+        debug("\nTable2 has a hidden first section. Verify accessing cells works as expected.");
+        var table2 = accessibilityController.accessibleElementById("table2");
+        shouldBe("table2.rowCount", "2");
+        var table2cell1 = table2.cellForColumnAndRow(0, 0);
+        shouldBeTrue("!table2cell1 || !table2cell1.isValid");
+        var table2cell2 = table2.cellForColumnAndRow(0, 1);
+        shouldBeTrue("table2cell2.isEqual(accessibilityController.accessibleElementById('table2cell'))");
+        shouldBe("table2cell2.rowIndexRange()", "'{1, 1}'");
+        shouldBe("table2cell2.columnIndexRange()", "'{0, 1}'");
+
+        debug("\nTable3 only has a footer section.");
+        var table3 = accessibilityController.accessibleElementById("table3");
+        shouldBe("table3.rowCount", "3");
+        var table3cell1 = table3.cellForColumnAndRow(0, 0);
+        shouldBeTrue("!table3cell1 || !table3cell1.isValid");
+        var table3cell2 = table3.cellForColumnAndRow(0, 2);
+        shouldBeTrue("table3cell2.isEqual(accessibilityController.accessibleElementById('table3cell'))");
+        shouldBe("table3cell2.rowIndexRange()", "'{2, 1}'");
+        shouldBe("table3cell2.columnIndexRange()", "'{0, 1}'");
+      
+        // Clear the HTML for better results.
+        document.getElementById("content").innerHTML = "";  
+    }
+
+
+</script>
+
+<script src="../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
+
index e8e2511..7f281d5 100644 (file)
@@ -1436,6 +1436,7 @@ webkit.org/b/109056 accessibility/html-html-element-is-ignored.html [ Skip ]
 
 webkit.org/b/73912 accessibility/aria-checkbox-sends-notification.html [ Failure Pass ]
 #webkit.org/b/98787 accessibility/aria-hidden-negates-no-visibility.html [ Skip ]
+webkit.org/b/110508 accessibility/table-with-hidden-head-section.html [ Skip ]
 webkit.org/b/111062 [ Linux Win Debug ] accessibility/svg-remote-element.html [ Failure ]
 
 # -----------------------------------------------------------------
index 8d1b9d5..2086fbd 100644 (file)
@@ -1,3 +1,58 @@
+2013-03-04  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: cellForColumnAndRow fails for tables with hidden table cells
+        https://bugs.webkit.org/show_bug.cgi?id=110050
+
+        Reviewed by Tim Horton.
+
+        If a table had hidden cells, then accessibility code was being confused in a few ways.
+        1) The cellForColumnAndRow method would return the wrong information since that was
+           using the RenderTableSection to retrieve a cell, which did not have the same data as the AXTable
+        2) The way we were adding children made it impossible to determine column and row range because we 
+           would skip rows that had hidden children
+        3) AccessibilityARIAGrid and AccessibilityTable were using different methods for cellForColumnAndRow
+
+        The fix does a few things to make things right:
+        1) Always add an accessibility row, even if there are no visible cells in that row.
+        2) Have one method for AXTable and AXARIAGrid for cellForColumnAndRow.
+        3) Change cellForColumnAndRow to query the accessibility children rather than the RenderTableSection in determining the row, col info.
+        4) cellForColumnAndRow should use unsigned values instead of int values.
+
+        Test: accessibility/table-with-hidden-head-section.html
+
+        * accessibility/AccessibilityARIAGrid.cpp:
+        (WebCore):
+        * accessibility/AccessibilityARIAGrid.h:
+        (AccessibilityARIAGrid):
+        * accessibility/AccessibilityARIAGridCell.cpp:
+        (WebCore::AccessibilityARIAGridCell::rowIndexRange):
+        (WebCore::AccessibilityARIAGridCell::columnIndexRange):
+        * accessibility/AccessibilityARIAGridCell.h:
+        (AccessibilityARIAGridCell):
+        * accessibility/AccessibilityTable.cpp:
+        (WebCore::AccessibilityTable::addChildren):
+        (WebCore::AccessibilityTable::cellForColumnAndRow):
+        * accessibility/AccessibilityTable.h:
+        (WebCore):
+        (AccessibilityTable):
+        * accessibility/AccessibilityTableCell.cpp:
+        (WebCore::AccessibilityTableCell::rowIndexRange):
+        (WebCore::AccessibilityTableCell::columnIndexRange):
+        * accessibility/AccessibilityTableCell.h:
+        (AccessibilityTableCell):
+        * accessibility/atk/WebKitAccessibleInterfaceTable.cpp:
+        (webkitAccessibleTableGetColumnAtIndex):
+        (webkitAccessibleTableGetRowAtIndex):
+        (webkitAccessibleTableGetColumnExtentAt):
+        (webkitAccessibleTableGetRowExtentAt):
+        (webkitAccessibleTableGetColumnHeader):
+        (webkitAccessibleTableGetRowHeader):
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
+        * rendering/RenderTableSection.h:
+        (RenderTableSection):
+        (WebCore::RenderTableSection::rowRendererAt):
+
 2013-03-04  Arpita Bahuguna  <a.bah@samsung.com>
 
         getAttribute does not behave correctly for mixed-case attributes on HTML elements
index 1f7d081..8664ac7 100644 (file)
@@ -132,50 +132,4 @@ void AccessibilityARIAGrid::addChildren()
         m_children.append(headerContainerObject);
 }
     
-AccessibilityTableCell* AccessibilityARIAGrid::cellForColumnAndRow(unsigned column, unsigned row)
-{
-    if (!m_renderer)
-        return 0;
-    
-    updateChildrenIfNecessary();
-    
-    if (column >= columnCount() || row >= rowCount())
-        return 0;
-    
-    int intRow = (int)row;
-    int intColumn = (int)column;
-
-    pair<int, int> columnRange;
-    pair<int, int> rowRange;
-    
-    // Iterate backwards through the rows in case the desired cell has a rowspan and exists
-    // in a previous row.
-    for (; intRow >= 0; --intRow) {
-        AccessibilityObject* tableRow = m_rows[intRow].get();
-        if (!tableRow)
-            continue;
-        
-        AccessibilityChildrenVector children = tableRow->children();
-        unsigned childrenLength = children.size();
-        
-        // Since some cells may have colspans, we have to check the actual range of each
-        // cell to determine which is the right one.
-        for (unsigned k = 0; k < childrenLength; ++k) {
-            AccessibilityObject* child = children[k].get();
-            if (!child->isTableCell()) 
-                continue;
-            
-            AccessibilityTableCell* tableCellChild = static_cast<AccessibilityTableCell*>(child);
-            tableCellChild->columnIndexRange(columnRange);
-            tableCellChild->rowIndexRange(rowRange);
-            
-            if ((intColumn >= columnRange.first && intColumn < (columnRange.first + columnRange.second))
-                && (intRow >= rowRange.first && intRow < (rowRange.first + rowRange.second)))
-                return tableCellChild;
-        }
-    }
-
-    return 0;
-}
-    
 } // namespace WebCore
index d3f8297..38f104d 100644 (file)
@@ -49,8 +49,6 @@ public:
     
     virtual void addChildren();
     
-    virtual AccessibilityTableCell* cellForColumnAndRow(unsigned column, unsigned row);
-
 private:
     // ARIA treegrids and grids support selected rows.
     virtual bool supportsSelectedRows() { return true; }    
index 55d09ad..9c3b167 100644 (file)
@@ -70,7 +70,7 @@ AccessibilityObject* AccessibilityARIAGridCell::parentTable() const
     return parent;
 }
     
-void AccessibilityARIAGridCell::rowIndexRange(pair<int, int>& rowRange)
+void AccessibilityARIAGridCell::rowIndexRange(pair<unsigned, unsigned>& rowRange)
 {
     AccessibilityObject* parent = parentObjectUnignored();
     if (!parent)
@@ -100,7 +100,7 @@ void AccessibilityARIAGridCell::rowIndexRange(pair<int, int>& rowRange)
     rowRange.second = 1;
 }
 
-void AccessibilityARIAGridCell::columnIndexRange(pair<int, int>& columnRange)
+void AccessibilityARIAGridCell::columnIndexRange(pair<unsigned, unsigned>& columnRange)
 {
     AccessibilityObject* parent = parentObjectUnignored();
     if (!parent)
index 53efa48..d5ae4ac 100644 (file)
@@ -42,9 +42,9 @@ public:
     virtual ~AccessibilityARIAGridCell();
     
     // fills in the start location and row span of cell
-    virtual void rowIndexRange(pair<int, int>& rowRange);
+    virtual void rowIndexRange(pair<unsigned, unsigned>& rowRange);
     // fills in the start location and column span of cell
-    virtual void columnIndexRange(pair<int, int>& columnRange);
+    virtual void columnIndexRange(pair<unsigned, unsigned>& columnRange);
     
 protected:
     virtual AccessibilityObject* parentTable() const;
index 653c831..4c33236 100644 (file)
@@ -344,53 +344,43 @@ void AccessibilityTable::addChildren()
     RenderTable* table = toRenderTable(m_renderer);
     AXObjectCache* axCache = m_renderer->document()->axObjectCache();
 
-    // go through all the available sections to pull out the rows
-    // and add them as children
-    // FIXME: This will skip a table with just a tfoot. Should fix by using RenderTable::topSection.
-    RenderTableSection* tableSection = table->header();
-    if (!tableSection)
-        tableSection = table->firstBody();
-    
+    // Go through all the available sections to pull out the rows and add them as children.
+    RenderTableSection* tableSection = table->topSection();
     if (!tableSection)
         return;
     
     RenderTableSection* initialTableSection = tableSection;
-    
     while (tableSection) {
         
         HashSet<AccessibilityObject*> appendedRows;
-
         unsigned numRows = tableSection->numRows();
-        unsigned numCols = tableSection->numColumns();
         for (unsigned rowIndex = 0; rowIndex < numRows; ++rowIndex) {
-            for (unsigned colIndex = 0; colIndex < numCols; ++colIndex) {
-                
-                RenderTableCell* cell = tableSection->primaryCellAt(rowIndex, colIndex);
-                if (!cell)
-                    continue;
-                
-                AccessibilityObject* rowObject = axCache->getOrCreate(cell->parent());
-                if (!rowObject->isTableRow())
-                    continue;
-                
-                AccessibilityTableRow* row = static_cast<AccessibilityTableRow*>(rowObject);
-                // we need to check every cell for a new row, because cell spans
-                // can cause us to mess rows if we just check the first column
-                if (appendedRows.contains(row))
-                    continue;
-                
-                row->setRowIndex((int)m_rows.size());        
-                m_rows.append(row);
-                if (!row->accessibilityIsIgnored())
-                    m_children.append(row);
+            
+            RenderTableRow* renderRow = tableSection->rowRendererAt(rowIndex);
+            if (!renderRow)
+                continue;
+            
+            AccessibilityObject* rowObject = axCache->getOrCreate(renderRow);
+            if (!rowObject->isTableRow())
+                continue;
+            
+            AccessibilityTableRow* row = static_cast<AccessibilityTableRow*>(rowObject);
+            // We need to check every cell for a new row, because cell spans
+            // can cause us to miss rows if we just check the first column.
+            if (appendedRows.contains(row))
+                continue;
+            
+            row->setRowIndex(static_cast<int>(m_rows.size()));
+            m_rows.append(row);
+            if (!row->accessibilityIsIgnored())
+                m_children.append(row);
 #if PLATFORM(GTK)
-                else
-                    m_children.append(row->children());
+            else
+                m_children.append(row->children());
 #endif
-                appendedRows.add(row);
-            }
+            appendedRows.add(row);
         }
-        
+    
         tableSection = table->sectionBelow(tableSection, SkipEmptySections);
     }
     
@@ -509,76 +499,36 @@ int AccessibilityTable::tableLevel() const
 
 AccessibilityTableCell* AccessibilityTable::cellForColumnAndRow(unsigned column, unsigned row)
 {
-    if (!m_renderer || !m_renderer->isTable())
-        return 0;
-    
     updateChildrenIfNecessary();
+    if (column >= columnCount() || row >= rowCount())
+        return 0;
     
-    RenderTable* table = toRenderTable(m_renderer);
-    // FIXME: This will skip a table with just a tfoot. Should fix by using RenderTable::topSection.
-    RenderTableSection* tableSection = table->header();
-    if (!tableSection)
-        tableSection = table->firstBody();
-    
-    RenderTableCell* cell = 0;
-    unsigned rowCount = 0;
-    unsigned rowOffset = 0;
-    while (tableSection) {
-        
-        unsigned numRows = tableSection->numRows();
-        unsigned numCols = tableSection->numColumns();
-        
-        rowCount += numRows;
-        
-        unsigned sectionSpecificRow = row - rowOffset;            
-        if (row < rowCount && column < numCols && sectionSpecificRow < numRows) {
-            cell = tableSection->primaryCellAt(sectionSpecificRow, column);
+    // Iterate backwards through the rows in case the desired cell has a rowspan and exists in a previous row.
+    for (unsigned rowIndexCounter = row + 1; rowIndexCounter > 0; --rowIndexCounter) {
+        unsigned rowIndex = rowIndexCounter - 1;
+        AccessibilityChildrenVector children = m_rows[rowIndex]->children();
+        // Since some cells may have colspans, we have to check the actual range of each
+        // cell to determine which is the right one.
+        for (unsigned colIndexCounter = std::min(static_cast<unsigned>(children.size()), column + 1); colIndexCounter > 0; --colIndexCounter) {
+            unsigned colIndex = colIndexCounter - 1;
+            AccessibilityObject* child = children[colIndex].get();
+            ASSERT(child->isTableCell());
+            if (!child->isTableCell())
+                continue;
             
-            // we didn't find the cell, which means there's spanning happening
-            // search backwards to find the spanning cell
-            if (!cell) {
-                
-                // first try rows
-                for (int testRow = sectionSpecificRow - 1; testRow >= 0; --testRow) {
-                    cell = tableSection->primaryCellAt(testRow, column);
-                    // cell overlapped. use this one
-                    ASSERT(!cell || cell->rowSpan() >= 1);
-                    if (cell && ((cell->rowIndex() + (cell->rowSpan() - 1)) >= sectionSpecificRow))
-                        break;
-                    cell = 0;
-                }
-                
-                if (!cell) {
-                    // try cols
-                    for (int testCol = column - 1; testCol >= 0; --testCol) {
-                        cell = tableSection->primaryCellAt(sectionSpecificRow, testCol);
-                        // cell overlapped. use this one
-                        ASSERT(!cell || cell->rowSpan() >= 1);
-                        if (cell && ((cell->col() + (cell->colSpan() - 1)) >= column))
-                            break;
-                        cell = 0;
-                    }
-                }
-            }
+            pair<unsigned, unsigned> columnRange;
+            pair<unsigned, unsigned> rowRange;
+            AccessibilityTableCell* tableCellChild = static_cast<AccessibilityTableCell*>(child);
+            tableCellChild->columnIndexRange(columnRange);
+            tableCellChild->rowIndexRange(rowRange);
+            
+            if ((column >= columnRange.first && column < (columnRange.first + columnRange.second))
+                && (row >= rowRange.first && row < (rowRange.first + rowRange.second)))
+                return tableCellChild;
         }
-        
-        if (cell)
-            break;
-        
-        rowOffset += numRows;
-        // we didn't find anything between the rows we should have
-        if (row < rowCount)
-            break;
-        tableSection = table->sectionBelow(tableSection, SkipEmptySections);
     }
     
-    if (!cell)
-        return 0;
-    
-    AccessibilityObject* cellObject = axObjectCache()->getOrCreate(cell);
-    ASSERT_WITH_SECURITY_IMPLICATION(cellObject->isTableCell());
-    
-    return static_cast<AccessibilityTableCell*>(cellObject);
+    return 0;
 }
 
 AccessibilityRole AccessibilityTable::roleValue() const
index f79d0ca..320a925 100644 (file)
@@ -35,6 +35,7 @@
 namespace WebCore {
 
 class AccessibilityTableCell;
+class RenderTableSection;
     
 class AccessibilityTable : public AccessibilityRenderObject {
 
@@ -67,7 +68,7 @@ public:
     
     // all the cells in the table
     void cells(AccessibilityChildrenVector&);
-    virtual AccessibilityTableCell* cellForColumnAndRow(unsigned column, unsigned row);
+    AccessibilityTableCell* cellForColumnAndRow(unsigned column, unsigned row);
     
     void columnHeaders(AccessibilityChildrenVector&);
     void rowHeaders(AccessibilityChildrenVector&);
index 31b9f68..0e78b36 100644 (file)
@@ -102,7 +102,7 @@ AccessibilityRole AccessibilityTableCell::determineAccessibilityRole()
     return CellRole;
 }
     
-void AccessibilityTableCell::rowIndexRange(pair<int, int>& rowRange)
+void AccessibilityTableCell::rowIndexRange(pair<unsigned, unsigned>& rowRange)
 {
     if (!m_renderer || !m_renderer->isTableCell())
         return;
@@ -117,11 +117,7 @@ void AccessibilityTableCell::rowIndexRange(pair<int, int>& rowRange)
     if (!table || !section)
         return;
 
-    // FIXME: This will skip a table with just a tfoot. Should fix by using RenderTable::topSection.
-    RenderTableSection* tableSection = table->header();
-    if (!tableSection)
-        tableSection = table->firstBody();
-    
+    RenderTableSection* tableSection = table->topSection();    
     unsigned rowOffset = 0;
     while (tableSection) {
         if (tableSection == section)
@@ -133,7 +129,7 @@ void AccessibilityTableCell::rowIndexRange(pair<int, int>& rowRange)
     rowRange.first += rowOffset;
 }
     
-void AccessibilityTableCell::columnIndexRange(pair<int, int>& columnRange)
+void AccessibilityTableCell::columnIndexRange(pair<unsigned, unsigned>& columnRange)
 {
     if (!m_renderer || !m_renderer->isTableCell())
         return;
index 72e5fd9..78f0b90 100644 (file)
@@ -44,9 +44,9 @@ public:
     virtual bool isTableCell() const;
     
     // fills in the start location and row span of cell
-    virtual void rowIndexRange(pair<int, int>& rowRange);
+    virtual void rowIndexRange(pair<unsigned, unsigned>& rowRange);
     // fills in the start location and column span of cell
-    virtual void columnIndexRange(pair<int, int>& columnRange);
+    virtual void columnIndexRange(pair<unsigned, unsigned>& columnRange);
     
 protected:
     virtual AccessibilityObject* parentTable() const;
index aee7b7b..9b65100 100644 (file)
@@ -115,7 +115,7 @@ static gint webkitAccessibleTableGetColumnAtIndex(AtkTable* table, gint index)
 {
     AccessibilityTableCell* axCell = cellAtIndex(table, index);
     if (axCell) {
-        pair<int, int> columnRange;
+        pair<unsigned, unsigned> columnRange;
         axCell->columnIndexRange(columnRange);
         return columnRange.first;
     }
@@ -126,7 +126,7 @@ static gint webkitAccessibleTableGetRowAtIndex(AtkTable* table, gint index)
 {
     AccessibilityTableCell* axCell = cellAtIndex(table, index);
     if (axCell) {
-        pair<int, int> rowRange;
+        pair<unsigned, unsigned> rowRange;
         axCell->rowIndexRange(rowRange);
         return rowRange.first;
     }
@@ -153,7 +153,7 @@ static gint webkitAccessibleTableGetColumnExtentAt(AtkTable* table, gint row, gi
 {
     AccessibilityTableCell* axCell = cell(table, row, column);
     if (axCell) {
-        pair<int, int> columnRange;
+        pair<unsigned, unsigned> columnRange;
         axCell->columnIndexRange(columnRange);
         return columnRange.second;
     }
@@ -164,7 +164,7 @@ static gint webkitAccessibleTableGetRowExtentAt(AtkTable* table, gint row, gint
 {
     AccessibilityTableCell* axCell = cell(table, row, column);
     if (axCell) {
-        pair<int, int> rowRange;
+        pair<unsigned, unsigned> rowRange;
         axCell->rowIndexRange(rowRange);
         return rowRange.second;
     }
@@ -179,10 +179,10 @@ static AtkObject* webkitAccessibleTableGetColumnHeader(AtkTable* table, gint col
         static_cast<AccessibilityTable*>(accTable)->columnHeaders(allColumnHeaders);
         unsigned columnCount = allColumnHeaders.size();
         for (unsigned k = 0; k < columnCount; ++k) {
-            pair<int, int> columnRange;
+            pair<unsigned, unsigned> columnRange;
             AccessibilityTableCell* cell = static_cast<AccessibilityTableCell*>(allColumnHeaders.at(k).get());
             cell->columnIndexRange(columnRange);
-            if (columnRange.first <= column && column < columnRange.first + columnRange.second)
+            if (columnRange.first <= static_cast<unsigned>(column) && static_cast<unsigned>(column) < columnRange.first + columnRange.second)
                 return allColumnHeaders[k]->wrapper();
         }
     }
@@ -197,10 +197,10 @@ static AtkObject* webkitAccessibleTableGetRowHeader(AtkTable* table, gint row)
         static_cast<AccessibilityTable*>(accTable)->rowHeaders(allRowHeaders);
         unsigned rowCount = allRowHeaders.size();
         for (unsigned k = 0; k < rowCount; ++k) {
-            pair<int, int> rowRange;
+            pair<unsigned, unsigned> rowRange;
             AccessibilityTableCell* cell = static_cast<AccessibilityTableCell*>(allRowHeaders.at(k).get());
             cell->rowIndexRange(rowRange);
-            if (rowRange.first <= row && row < rowRange.first + rowRange.second)
+            if (rowRange.first <= static_cast<unsigned>(row) && static_cast<unsigned>(row) < rowRange.first + rowRange.second)
                 return allRowHeaders[k]->wrapper();
         }
     }
index bd70f51..6b04f24 100644 (file)
@@ -2381,12 +2381,12 @@ static NSString* roleValueToNSString(AccessibilityRole value)
     
     if (m_object->isTableCell()) {
         if ([attributeName isEqualToString:NSAccessibilityRowIndexRangeAttribute]) {
-            pair<int, int> rowRange;
+            pair<unsigned, unsigned> rowRange;
             static_cast<AccessibilityTableCell*>(m_object)->rowIndexRange(rowRange);
             return [NSValue valueWithRange:NSMakeRange(rowRange.first, rowRange.second)];
         }
         if ([attributeName isEqualToString:NSAccessibilityColumnIndexRangeAttribute]) {
-            pair<int, int> columnRange;
+            pair<unsigned, unsigned> columnRange;
             static_cast<AccessibilityTableCell*>(m_object)->columnIndexRange(columnRange);
             return [NSValue valueWithRange:NSMakeRange(columnRange.first, columnRange.second)];
         }
index 8903b28..f55e9e2 100644 (file)
@@ -153,6 +153,8 @@ public:
         return c.primaryCell();
     }
 
+    RenderTableRow* rowRendererAt(unsigned row) const { return m_grid[row].rowRenderer; }
+
     void appendColumn(unsigned pos);
     void splitColumn(unsigned pos, unsigned first);
 
index 460a819..0e5a6d6 100644 (file)
@@ -1,3 +1,16 @@
+2013-03-04  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: cellForColumnAndRow fails for tables with hidden table cells
+        https://bugs.webkit.org/show_bug.cgi?id=110050
+
+        Reviewed by Tim Horton.
+
+        * src/WebAccessibilityObject.cpp:
+        (WebKit::WebAccessibilityObject::cellColumnIndex):
+        (WebKit::WebAccessibilityObject::cellColumnSpan):
+        (WebKit::WebAccessibilityObject::cellRowIndex):
+        (WebKit::WebAccessibilityObject::cellRowSpan):
+
 2013-03-04  Kunihiko Sakamoto  <ksakamoto@chromium.org>
 
         [Chromium] Add runtime flag for font load events
index abd4d56..50b25fd 100644 (file)
@@ -899,7 +899,7 @@ unsigned WebAccessibilityObject::cellColumnIndex() const
     if (!m_private->isTableCell())
        return 0;
 
-    pair<int, int> columnRange;
+    pair<unsigned, unsigned> columnRange;
     static_cast<WebCore::AccessibilityTableCell*>(m_private.get())->columnIndexRange(columnRange);
     return columnRange.first;
 }
@@ -912,7 +912,7 @@ unsigned WebAccessibilityObject::cellColumnSpan() const
     if (!m_private->isTableCell())
        return 0;
 
-    pair<int, int> columnRange;
+    pair<unsigned, unsigned> columnRange;
     static_cast<WebCore::AccessibilityTableCell*>(m_private.get())->columnIndexRange(columnRange);
     return columnRange.second;
 }
@@ -925,7 +925,7 @@ unsigned WebAccessibilityObject::cellRowIndex() const
     if (!m_private->isTableCell())
        return 0;
 
-    pair<int, int> rowRange;
+    pair<unsigned, unsigned> rowRange;
     static_cast<WebCore::AccessibilityTableCell*>(m_private.get())->rowIndexRange(rowRange);
     return rowRange.first;
 }
@@ -938,7 +938,7 @@ unsigned WebAccessibilityObject::cellRowSpan() const
     if (!m_private->isTableCell())
        return 0;
 
-    pair<int, int> rowRange;
+    pair<unsigned, unsigned> rowRange;
     static_cast<WebCore::AccessibilityTableCell*>(m_private.get())->rowIndexRange(rowRange);
     return rowRange.second;
 }