Changeset 96340 in webkit

Timestamp:

Sep 29, 2011 10:46:14 AM (13 years ago)

Author:

Chris Fleizach

Message:

ARIA live regions don't trigger notifications for elements that aren't in the AX tree
​https://bugs.webkit.org/show_bug.cgi?id=62289

Source/WebCore:

If an ARIA Live region udpates an element that is not in the AX object cache, then the Live region
notification is not sent. To fix this, the childrenChanged() method needs to actually create
the appropriate objects, but since that method gets called during a render tree update, we've learned
that it's generally not safe to create objects.

Instead a one shot timer can be fired that will update and create the necessary objects so that the
correct notification can be sent.

Reviewed by Darin Adler.

Test: platform/mac/accessibility/aria-liveregion-without-element-access.html

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::AXObjectCache):
(WebCore::AXObjectCache::~AXObjectCache):
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::childrenUpdateTimerFired):
(WebCore::AXObjectCache::childrenChanged):

• accessibility/AXObjectCache.h:
• accessibility/AccessibilityMenuList.cpp:

(WebCore::AccessibilityMenuList::childrenChanged):

• accessibility/AccessibilityMenuList.h:
• accessibility/AccessibilityMenuListPopup.cpp:

(WebCore::AccessibilityMenuListPopup::childrenChanged):

• accessibility/AccessibilityMenuListPopup.h:
• accessibility/AccessibilityObject.h:

(WebCore::AccessibilityObject::childrenChanged):

• accessibility/AccessibilityRenderObject.cpp:

(WebCore::startOfContinuations):
(WebCore::AccessibilityRenderObject::updateAccessibilityRole):
(WebCore::AccessibilityRenderObject::childrenChanged):

• accessibility/AccessibilityRenderObject.h:
• rendering/RenderObject.cpp:

(WebCore::RenderObject::willBeDestroyed):

LayoutTests:

Reviewed by Darin Adler.

• platform/mac/accessibility/aria-liveregion-without-element-access-expected.txt: Added.
• platform/mac/accessibility/aria-liveregion-without-element-access.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/accessibility/aria-liveregion-without-element-access-expected.txt (added)
• LayoutTests/platform/mac/accessibility/aria-liveregion-without-element-access.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AXObjectCache.cpp (modified) (5 diffs)
• Source/WebCore/accessibility/AXObjectCache.h (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityMenuList.cpp (modified) (2 diffs)
• Source/WebCore/accessibility/AccessibilityMenuList.h (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityMenuListPopup.h (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityObject.h (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityRenderObject.cpp (modified) (4 diffs)
• Source/WebCore/accessibility/AccessibilityRenderObject.h (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-29  Chris Fleizach  <cfleizach@apple.com>
+
+        ARIA live regions don't trigger notifications for elements that aren't in the AX tree
+        https://bugs.webkit.org/show_bug.cgi?id=62289
+
+        Reviewed by Darin Adler.
+
+        * platform/mac/accessibility/aria-liveregion-without-element-access-expected.txt: Added.
+        * platform/mac/accessibility/aria-liveregion-without-element-access.html: Added.
+
 2011-09-29  David Reveman  <reveman@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-29  Chris Fleizach  <cfleizach@apple.com>
+
+        ARIA live regions don't trigger notifications for elements that aren't in the AX tree
+        https://bugs.webkit.org/show_bug.cgi?id=62289
+
+        If an ARIA Live region udpates an element that is not in the AX object cache, then the Live region
+        notification is not sent. To fix this, the childrenChanged() method needs to actually create
+        the appropriate objects, but since that method gets called during a render tree update, we've learned 
+        that it's generally not safe to create objects.
+
+        Instead a one shot timer can be fired that will update and create the necessary objects so that the
+        correct notification can be sent.
+
+        Reviewed by Darin Adler.
+
+        Test: platform/mac/accessibility/aria-liveregion-without-element-access.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::AXObjectCache):
+        (WebCore::AXObjectCache::~AXObjectCache):
+        (WebCore::AXObjectCache::remove):
+        (WebCore::AXObjectCache::childrenUpdateTimerFired):
+        (WebCore::AXObjectCache::childrenChanged):
+        * accessibility/AXObjectCache.h:
+        * accessibility/AccessibilityMenuList.cpp:
+        (WebCore::AccessibilityMenuList::childrenChanged):
+        * accessibility/AccessibilityMenuList.h:
+        * accessibility/AccessibilityMenuListPopup.cpp:
+        (WebCore::AccessibilityMenuListPopup::childrenChanged):
+        * accessibility/AccessibilityMenuListPopup.h:
+        * accessibility/AccessibilityObject.h:
+        (WebCore::AccessibilityObject::childrenChanged):
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::startOfContinuations):
+        (WebCore::AccessibilityRenderObject::updateAccessibilityRole):
+        (WebCore::AccessibilityRenderObject::childrenChanged):
+        * accessibility/AccessibilityRenderObject.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willBeDestroyed):
+
 2011-09-29  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -83 +83 @@
 AXObjectCache::AXObjectCache(const Document* doc)
     : m_notificationPostTimer(this, &AXObjectCache::notificationPostTimerFired)
+    , m_childrenUpdateTimer(this, &AXObjectCache::childrenUpdateTimerFired)
 {
     m_document = const_cast<Document*>(doc);
@@ -89 +90 @@
 AXObjectCache::~AXObjectCache()
 {
+    m_childrenUpdateTimer.stop();
+    m_notificationPostTimer.stop();
+    
     HashMap<AXID, RefPtr<AccessibilityObject> >::iterator end = m_objects.end();
     for (HashMap<AXID, RefPtr<AccessibilityObject> >::iterator it = m_objects.begin(); it != end; ++it) {
@@ -368 +372 @@
     remove(axID);
     m_renderObjectMapping.remove(renderer);
+    m_childrenToUpdate.remove(renderer);
 }
 
@@ -437 +442 @@
 }
 #endif
+    
+void AXObjectCache::childrenUpdateTimerFired(Timer<AXObjectCache>*)
+{
+    if (m_childrenToUpdate.isEmpty())
+        return;
+    
+    // Make a local copy in case childrenChanged() alters m_childrenToUpdate 
+    // (which might happen if the client asks to update the render tree).
+    HashSet<RenderObject*> updateChildren;
+    m_childrenToUpdate.swap(updateChildren);
+    m_childrenToUpdate.clear();
+    
+    HashSet<RenderObject*>::iterator end = updateChildren.end();
+    for (HashSet<RenderObject*>::iterator it = updateChildren.begin(); it != end; ++it) {
+        if (AccessibilityObject* object = getOrCreate(*it))
+            object->childrenChanged(AccessibilityObject::CreateParentObjects);
+    }    
+}
 
 void AXObjectCache::childrenChanged(RenderObject* renderer)
@@ -444 +467 @@
  
     AXID axID = m_renderObjectMapping.get(renderer);
-    if (!axID)
-        return;
-    
-    AccessibilityObject* obj = m_objects.get(axID).get();
-    if (obj)
-        obj->childrenChanged();
+    if (!axID) {
+        // If there's no AX object, creating one right now can be dangerous (because we're in the middle of adding/destroying a tree).
+        // Instead the update should be postponed and updated later.
+        m_childrenToUpdate.add(renderer);
+
+        if (!m_childrenUpdateTimer.isActive())
+            m_childrenUpdateTimer.startOneShot(0);
+    } else {
+        if (AccessibilityObject* object = m_objects.get(axID).get())
+            object->childrenChanged(AccessibilityObject::DoNotCreateParentObjects);
+    }
+
 }
     

trunk/Source/WebCore/accessibility/AXObjectCache.h

@@ -168 +168 @@
     Vector<pair<RefPtr<AccessibilityObject>, AXNotification> > m_notificationsToPost;
     void notificationPostTimerFired(Timer<AXObjectCache>*);
-    
+
+    Timer<AXObjectCache> m_childrenUpdateTimer;
+    HashSet<RenderObject*> m_childrenToUpdate;
+    void childrenUpdateTimerFired(Timer<AXObjectCache>*);
+
     static AccessibilityObject* focusedImageMapUIElement(HTMLAreaElement*);
     

trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp

@@ -69 +69 @@
 }
 
-void AccessibilityMenuList::childrenChanged()
+void AccessibilityMenuList::childrenChanged(ChildrenChangeOptions options)
 {
     if (m_children.isEmpty())
@@ -75 +75 @@
 
     ASSERT(m_children.size() == 1);
-    m_children[0]->childrenChanged();
+    m_children[0]->childrenChanged(options);
 }
 

trunk/Source/WebCore/accessibility/AccessibilityMenuList.h

@@ -54 +54 @@
 
     virtual void addChildren();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
 };
 

trunk/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp

@@ -104 +104 @@
 }
 
-void AccessibilityMenuListPopup::childrenChanged()
+void AccessibilityMenuListPopup::childrenChanged(ChildrenChangeOptions)
 {
     for (size_t i = m_children.size(); i > 0 ; --i) {

trunk/Source/WebCore/accessibility/AccessibilityMenuListPopup.h

@@ -60 +60 @@
     virtual bool press() const;
     virtual void addChildren();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
 
     AccessibilityMenuListOption* menuListOptionAccessibilityObject(HTMLElement*) const;

trunk/Source/WebCore/accessibility/AccessibilityObject.h

@@ -551 +551 @@
     virtual void decrement() { }
 
-    virtual void childrenChanged() { }
+    enum ChildrenChangeOptions { DoNotCreateParentObjects, CreateParentObjects };
+    virtual void childrenChanged(ChildrenChangeOptions) { }
     virtual void contentChanged() { }
     virtual const AccessibilityChildrenVector& children() { return m_children; }

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -224 +224 @@
 static inline RenderInline* startOfContinuations(RenderObject* r)
 {
-    if (r->isInlineElementContinuation())
+    if (r->isInlineElementContinuation() && r->node()->renderer() && r->isRenderInline())
         return toRenderInline(r->node()->renderer());
 
@@ -3088 +3088 @@
     // The AX hierarchy only needs to be updated if the ignored status of an element has changed.
     if (ignoredStatus != accessibilityIsIgnored())
-        childrenChanged();
+        childrenChanged(DoNotCreateParentObjects);
 }
     
@@ -3371 +3371 @@
 }
     
-void AccessibilityRenderObject::childrenChanged()
+void AccessibilityRenderObject::childrenChanged(ChildrenChangeOptions options)
 {
     // This method is meant as a quick way of marking a portion of the accessibility tree dirty.
@@ -3383 +3383 @@
     // If AX elements are created now, they could interrogate the render tree while it's in a funky state.
     // At the same time, process ARIA live region changes.
-    for (AccessibilityObject* parent = this; parent; parent = parent->parentObjectIfExists()) {
+    for (AccessibilityObject* parent = this; parent; parent = (options == CreateParentObjects) ? parent->parentObject() : parent->parentObjectIfExists()) {
         if (!parent->isAccessibilityRenderObject())
             continue;

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h

@@ -216 +216 @@
     
     virtual void detach();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
     virtual void contentChanged();
     virtual void addChildren();

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -2177 +2177 @@
         frame()->eventHandler()->stopAutoscrollTimer(true);
 
-    if (AXObjectCache::accessibilityEnabled()) {
-        document()->axObjectCache()->childrenChanged(this->parent());
-        document()->axObjectCache()->remove(this);
-    }
     animation()->cancelAnimations(this);
 
@@ -2198 +2194 @@
         setHasLayer(false);
         toRenderBoxModelObject(this)->destroyLayer();
+    }
+    
+    // Update accessibility at the end, so that all children nodes have been disassociated first.
+    // This ordering allows us to call childrenChanged() on the parent without worrying that the parent has been destroyed.
+    if (AXObjectCache::accessibilityEnabled()) {
+        document()->axObjectCache()->childrenChanged(this->parent());
+        document()->axObjectCache()->remove(this);
     }
 }