Changeset 40098 in webkit

Timestamp:

Jan 21, 2009 2:55:01 PM (15 years ago)

Author:

Chris Fleizach

Message:

Bug 23443: Table accessibility should be re-enabled after fixing crash that occurs at WebCore::AccessibilityTable::isTableExposableThroughAccessibility() when attempting to create a link in a rich text message
​https://bugs.webkit.org/show_bug.cgi?id=23443

Re-enable Accessibility tables and make sure accessibility code does not interrogate the render tree
during render tree updates

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/table-modification-crash-expected.txt (added)
• LayoutTests/accessibility/table-modification-crash.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/AccessibilityObject.cpp (modified) (1 diff)
• WebCore/page/AccessibilityObject.h (modified) (1 diff)
• WebCore/page/AccessibilityRenderObject.cpp (modified) (3 diffs)
• WebCore/page/AccessibilityRenderObject.h (modified) (2 diffs)
• WebCore/page/AccessibilityTable.cpp (modified) (1 diff)
• WebCore/page/mac/AccessibilityObjectWrapper.mm (modified) (15 diffs)
• WebCore/rendering/RenderObject.cpp (modified) (1 diff)
• WebCore/rendering/RenderWidget.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-01-21  Chris Fleizach  <cfleizach@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        Test to make sure accessibility doesn't crash when a table is modified through JavaScript
+
+        * accessibility/table-modification-crash-expected.txt: Added.
+        * accessibility/table-modification-crash.html: Added.
+
 2009-01-16  Eric Seidel  <eric@webkit.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-01-21  Chris Fleizach  <cfleizach@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        Bug 23443: Table accessibility should be re-enabled after fixing crash that occurs at WebCore::AccessibilityTable::isTableExposableThroughAccessibility() 
+        https://bugs.webkit.org/show_bug.cgi?id=23443
+
+        Test: accessibility/table-modification-crash.html
+
+        * page/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::updateBackingStore):
+        * page/AccessibilityObject.h:
+        * page/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::childrenChanged):
+        (WebCore::AccessibilityRenderObject::children):
+        (WebCore::AccessibilityRenderObject::updateBackingStore):
+        * page/AccessibilityRenderObject.h:
+        (WebCore::AccessibilityRenderObject::markChildrenDirty):
+        * page/AccessibilityTable.cpp:
+        (WebCore::AccessibilityTable::AccessibilityTable):
+        * page/mac/AccessibilityObjectWrapper.mm:
+        (-[AccessibilityObjectWrapper accessibilityActionNames]):
+        (-[AccessibilityObjectWrapper accessibilityAttributeNames]):
+        (-[AccessibilityObjectWrapper accessibilityAttributeValue:]):
+        (-[AccessibilityObjectWrapper accessibilityFocusedUIElement]):
+        (-[AccessibilityObjectWrapper accessibilityHitTest:]):
+        (-[AccessibilityObjectWrapper accessibilityIsAttributeSettable:]):
+        (-[AccessibilityObjectWrapper accessibilityIsIgnored]):
+        (-[AccessibilityObjectWrapper accessibilityParameterizedAttributeNames]):
+        (-[AccessibilityObjectWrapper accessibilityPerformPressAction]):
+        (-[AccessibilityObjectWrapper accessibilityPerformAction:]):
+        (-[AccessibilityObjectWrapper accessibilitySetValue:forAttribute:]):
+        (-[AccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):
+        (-[AccessibilityObjectWrapper accessibilityIndexOfChild:]):
+        (-[AccessibilityObjectWrapper accessibilityArrayAttributeCount:]):
+        (-[AccessibilityObjectWrapper accessibilityArrayAttributeValues:index:maxCount:]):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::destroy):
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::destroy):
+
 2009-01-16  Eric Seidel  <eric@webkit.org>
 

trunk/WebCore/page/AccessibilityObject.cpp

@@ -1029 +1029 @@
 }
 
+void AccessibilityObject::updateBackingStore()
+{
+}
+    
 } // namespace WebCore

trunk/WebCore/page/AccessibilityObject.h

@@ -401 +401 @@
 #endif
 
+    // allows for an AccessibilityObject to update its render tree or perform
+    // other operations update type operations
+    virtual void updateBackingStore();
+    
 protected:
     unsigned m_id;

trunk/WebCore/page/AccessibilityRenderObject.cpp

@@ -2216 +2216 @@
 void AccessibilityRenderObject::childrenChanged()
 {
-    clearChildren();
-    
-    if (accessibilityIsIgnored()) {
-        AccessibilityObject* parent = parentObject();
-        if (parent)
-            parent->childrenChanged();
+    // this method is meant as a quick way of marking dirty
+    // a portion of the accessibility tree
+    
+    markChildrenDirty();
+    
+    // this object may not be accessible (and thus may not appear
+    // in the hierarchy), which means we need to go up the parent
+    // chain and mark the parent's dirty. Ideally, we would want
+    // to only access the next object that is not ignored, but
+    // asking an element if it's ignored can lead to an examination of the
+    // render tree which is dangerous.
+    for (AccessibilityObject* parent = parentObject(); parent; parent = parent->parentObject()) {
+        if (parent->isAccessibilityRenderObject())
+            static_cast<AccessibilityRenderObject *>(parent)->markChildrenDirty();
     }
 }
@@ -2245 +2253 @@
 const AccessibilityObject::AccessibilityChildrenVector& AccessibilityRenderObject::children()
 {
+    if (m_childrenDirty) {
+        clearChildren();        
+        m_childrenDirty = false;
+    }
+    
     if (!m_haveChildren)
         addChildren();
@@ -2405 +2418 @@
 }
      
+void AccessibilityRenderObject::updateBackingStore()
+{
+    if (!m_renderer)
+        return;
+    m_renderer->view()->layoutIfNeeded();
+}    
     
 } // namespace WebCore

trunk/WebCore/page/AccessibilityRenderObject.h

@@ -210 +210 @@
     virtual IntRect doAXBoundsForRange(const PlainTextRange&) const;
     
+    virtual void updateBackingStore();
+    
 protected:
     RenderObject* m_renderer;
     AccessibilityRole m_ariaRole;
+    mutable bool m_childrenDirty;
     
     void setRenderObject(RenderObject* renderer) { m_renderer = renderer; }
@@ -232 +235 @@
     AccessibilityObject* accessibilityParentForImageMap(HTMLMapElement* map) const;
 
+    void markChildrenDirty() const { m_childrenDirty = true; }
 };
     

trunk/WebCore/page/AccessibilityTable.cpp

@@ -54 +54 @@
     m_headerContainer(0)
 {
-    // FIXME: We need to disable Accessibility Tables entirely on the Mac until <rdar://problem/6372481> is resolved.
-#if PLATFORM(MAC)
+#if defined(BUILDING_ON_TIGER) || defined(BUILDING_ON_LEOPARD)
     m_isAccessibilityTable = false;
-#else    
+#else
     m_isAccessibilityTable = isTableExposableThroughAccessibility();
 #endif
-
 }
 

trunk/WebCore/page/mac/AccessibilityObjectWrapper.mm

@@ -581 +581 @@
 - (NSArray*)accessibilityActionNames
 {
+    m_object->updateBackingStore();
+
     static NSArray* actionElementActions = [[NSArray alloc] initWithObjects: NSAccessibilityPressAction, NSAccessibilityShowMenuAction, nil];
     static NSArray* defaultElementActions = [[NSArray alloc] initWithObjects: NSAccessibilityShowMenuAction, nil];
@@ -600 +602 @@
 - (NSArray*)accessibilityAttributeNames
 {
+    m_object->updateBackingStore();
+    
     if (m_object->isAttachment())
         return [[self attachmentView] accessibilityAttributeNames];
@@ -1125 +1129 @@
         return nil;
 
+    m_object->updateBackingStore();
+    
     if ([attributeName isEqualToString: NSAccessibilityRoleAttribute])
         return [self role];
@@ -1431 +1437 @@
 - (id)accessibilityFocusedUIElement
 {
+    m_object->updateBackingStore();
+
     RefPtr<AccessibilityObject> focusedObj = m_object->focusedUIElement();
 
@@ -1441 +1449 @@
 - (id)accessibilityHitTest:(NSPoint)point
 {
+    m_object->updateBackingStore();
+
     RefPtr<AccessibilityObject> axObject = m_object->doAccessibilityHitTest(IntPoint(point));
     if (axObject)
@@ -1449 +1459 @@
 - (BOOL)accessibilityIsAttributeSettable:(NSString*)attributeName
 {
+    m_object->updateBackingStore();
+
     if ([attributeName isEqualToString: @"AXSelectedTextMarkerRange"])
         return YES;
@@ -1483 +1495 @@
 - (BOOL)accessibilityIsIgnored
 {
+    m_object->updateBackingStore();
+
     if (m_object->isAttachment())
         return [[self attachmentView] accessibilityIsIgnored];
@@ -1490 +1504 @@
 - (NSArray* )accessibilityParameterizedAttributeNames
 {
+    m_object->updateBackingStore();
+
     if (m_object->isAttachment()) 
         return nil;
@@ -1570 +1586 @@
 - (void)accessibilityPerformPressAction
 {
+    m_object->updateBackingStore();
+
     if (m_object->isAttachment())
         [[self attachmentView] accessibilityPerformAction:NSAccessibilityPressAction];
@@ -1614 +1632 @@
 - (void)accessibilityPerformAction:(NSString*)action
 {
+    m_object->updateBackingStore();
+
     if ([action isEqualToString:NSAccessibilityPressAction])
         [self accessibilityPerformPressAction];
@@ -1623 +1643 @@
 - (void)accessibilitySetValue:(id)value forAttribute:(NSString*)attributeName
 {
+    m_object->updateBackingStore();
+
     WebCoreTextMarkerRange* textMarkerRange = nil;
     NSNumber*               number = nil;
@@ -1745 +1767 @@
         return nil;
 
+    m_object->updateBackingStore();
+    
     // common parameter type check/casting.  Nil checks in handlers catch wrong type case.
     // NOTE: This assumes nil is not a valid parameter, because it is indistinguishable from
@@ -1990 +2014 @@
 - (NSUInteger)accessibilityIndexOfChild:(id)child
 {
+    m_object->updateBackingStore();
+    
     const AccessibilityObject::AccessibilityChildrenVector& children = m_object->children();
        
@@ -2006 +2032 @@
 - (NSUInteger)accessibilityArrayAttributeCount:(NSString *)attribute
 {
+    m_object->updateBackingStore();
+    
     if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) {
         const AccessibilityObject::AccessibilityChildrenVector& children = m_object->children();
@@ -2019 +2047 @@
 - (NSArray *)accessibilityArrayAttributeValues:(NSString *)attribute index:(NSUInteger)index maxCount:(NSUInteger)maxCount 
 {
+    m_object->updateBackingStore();
+    
     if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) {
         if (m_object->children().isEmpty()) {

trunk/WebCore/rendering/RenderObject.cpp

@@ -2623 +2623 @@
         RenderCounter::destroyCounterNodes(this);
 
-    if (AXObjectCache::accessibilityEnabled())
+    if (AXObjectCache::accessibilityEnabled()) {
+        document()->axObjectCache()->childrenChanged(this->parent());
         document()->axObjectCache()->remove(this);
-
+    }
     animation()->cancelAnimations(this);
 

trunk/WebCore/rendering/RenderWidget.cpp

@@ -76 +76 @@
         v->removeWidget(this);
 
-    if (AXObjectCache::accessibilityEnabled())
+    if (AXObjectCache::accessibilityEnabled()) {
+        document()->axObjectCache()->childrenChanged(this->parent());
         document()->axObjectCache()->remove(this);
-
+    }
     remove();