Changeset 140095 in webkit

Timestamp:

Jan 17, 2013 7:46:37 PM (11 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION (r139444): Crashes in three accessibility tests on GTK
​https://bugs.webkit.org/show_bug.cgi?id=106922

Patch by Joanmarie Diggs <​jdiggs@igalia.com> on 2013-01-17
Reviewed by Chris Fleizach.

Source/WebCore:

r139444 exposed an infinite loop that was just waiting to happen.
Currently, getting the role value of an ARIA table row involves
getting the parent table via parentObjectUnignored() which in turn
can lead to the role value of the table row being checked. Moving
the roleValue() logic to determineAccessibilityRole() avoids this.

This fix, however, introduced a regression when an accessible row
was examined without having first examined the parent table for
non-ARIA tables. Now that it is safe to call parentObjectUnignored(),
the parentTable() method used for ARIA table rows can also be used
by non-ARIA table rows.

No new tests; instead unskipped the three crashing tests.

• accessibility/AccessibilityTableRow.cpp:

(WebCore::AccessibilityTableRow::determineAccessibilityRole):

• accessibility/AccessibilityTableRow.h:

(AccessibilityTableRow):

LayoutTests:

• platform/gtk/TestExpectations: Unskipped the three failing tests

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityARIAGridRow.h (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityTableRow.cpp (modified) (2 diffs)
• Source/WebCore/accessibility/AccessibilityTableRow.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-17  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        REGRESSION (r139444): Crashes in three accessibility tests on GTK
+        https://bugs.webkit.org/show_bug.cgi?id=106922
+
+        Reviewed by Chris Fleizach.
+
+        * platform/gtk/TestExpectations: Unskipped the three failing tests
+
 2013-01-17  Elliott Sprehn  <esprehn@chromium.org>
 

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -484 +484 @@
 webkit.org/b/105689 [ Debug ] plugins/npruntime/embed-property-iframe-equality.html [ Crash ]
 
-webkit.org/b/106922 accessibility/aria-tables.html [ Crash ]
-webkit.org/b/106922 accessibility/aria-hidden-with-elements.html [ Crash ]
-webkit.org/b/106922 platform/gtk/accessibility/aria-table-hierarchy.html [ Crash ]
 
 #////////////////////////////////////////////////////////////////////////////////////////

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-17  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        REGRESSION (r139444): Crashes in three accessibility tests on GTK
+        https://bugs.webkit.org/show_bug.cgi?id=106922
+
+        Reviewed by Chris Fleizach.
+
+        r139444 exposed an infinite loop that was just waiting to happen.
+        Currently, getting the role value of an ARIA table row involves
+        getting the parent table via parentObjectUnignored() which in turn
+        can lead to the role value of the table row being checked. Moving
+        the roleValue() logic to determineAccessibilityRole() avoids this.
+
+        This fix, however, introduced a regression when an accessible row
+        was examined without having first examined the parent table for
+        non-ARIA tables. Now that it is safe to call parentObjectUnignored(),
+        the parentTable() method used for ARIA table rows can also be used
+        by non-ARIA table rows.
+
+        No new tests; instead unskipped the three crashing tests.
+
+        * accessibility/AccessibilityTableRow.cpp:
+        (WebCore::AccessibilityTableRow::determineAccessibilityRole):
+        * accessibility/AccessibilityTableRow.h:
+        (AccessibilityTableRow):
+
 2013-01-17  Rik Cabanier  <cabanier@adobe.com>
 

trunk/Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp

@@ -119 +119 @@
 }
     
-AccessibilityObject* AccessibilityARIAGridRow::parentTable() const
-{
-    AccessibilityObject* parent = parentObjectUnignored();
-    if (!parent->isAccessibilityTable())
-        return 0;
-    
-    return parent;
-}
-
 AccessibilityObject* AccessibilityARIAGridRow::headerObject()
 {

trunk/Source/WebCore/accessibility/AccessibilityARIAGridRow.h

@@ -46 +46 @@
     
     virtual AccessibilityObject* headerObject();
-    virtual AccessibilityObject* parentTable() const;    
     
 private:

trunk/Source/WebCore/accessibility/AccessibilityTableRow.cpp

@@ -60 +60 @@
 }
 
-AccessibilityRole AccessibilityTableRow::roleValue() const
+AccessibilityRole AccessibilityTableRow::determineAccessibilityRole()
 {
     if (!isTableRow())
-        return AccessibilityRenderObject::roleValue();
-    
+        return AccessibilityRenderObject::determineAccessibilityRole();
+
+    m_ariaRole = determineAriaRoleAttribute();
+
+    AccessibilityRole ariaRole = ariaRoleAttribute();
+    if (ariaRole != UnknownRole)
+        return ariaRole;
+
     return RowRole;
 }
@@ -99 +105 @@
 AccessibilityObject* AccessibilityTableRow::parentTable() const
 {
-    if (!m_renderer || !m_renderer->isTableRow())
+    AccessibilityObject* parent = parentObjectUnignored();
+    if (!parent || !parent->isAccessibilityTable())
         return 0;
     
-    // Do not use getOrCreate. parentTable() can be called while the render tree is being modified.
-    return axObjectCache()->get(toRenderTableRow(m_renderer)->table());
+    return parent;
 }
     

trunk/Source/WebCore/accessibility/AccessibilityTableRow.h

@@ -43 +43 @@
     
     virtual bool isTableRow() const;
-    virtual AccessibilityRole roleValue() const;
     virtual bool accessibilityIsIgnored() const;
 
@@ -57 +56 @@
     void appendChild(AccessibilityObject*);
     
+protected:
+    virtual AccessibilityRole determineAccessibilityRole();
+
 private:
     int m_rowIndex;