ARIA live regions don't trigger notifications for elements that aren't in the AX...
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Jun 2011 17:59:48 +0000 (17:59 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Jun 2011 17:59:48 +0000 (17:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=62289

Reviewed by Darin Adler.

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, I think 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.

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

* accessibility/AXObjectCache.cpp:
(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):
   This changed exposed a case where an object was inlineElementContinuation, but not renderInlined,
   which led to an assert.
(WebCore::AccessibilityRenderObject::updateAccessibilityRole):
(WebCore::AccessibilityRenderObject::childrenChanged):
* accessibility/AccessibilityRenderObject.h:

LayoutTests:

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

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

LayoutTests/ChangeLog
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/accessibility/AccessibilityMenuList.cpp
Source/WebCore/accessibility/AccessibilityMenuList.h
Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp
Source/WebCore/accessibility/AccessibilityMenuListPopup.h
Source/WebCore/accessibility/AccessibilityObject.h
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.h

index 7964194..a62db79 100644 (file)
@@ -1,3 +1,13 @@
+2011-06-23  Chris Fleizach  <cfleizach@apple.com>
+
+        Reviewed by Darin Adler.
+
+        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
+
+        * platform/mac/accessibility/aria-liveregion-without-element-access-expected.txt: Added.
+        * platform/mac/accessibility/aria-liveregion-without-element-access.html: Added.
+
 2011-06-23  Pavel Feldman  <pfeldman@google.com>
 
         Not reviewed: marking debugger test as slow in chromium expectations.
index 5a5dac9..be3a72f 100644 (file)
@@ -1,3 +1,42 @@
+2011-06-23  Chris Fleizach  <cfleizach@apple.com>
+
+        Reviewed by Darin Adler.
+
+        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, I think 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.
+
+        Test: platform/mac/accessibility/aria-liveregion-without-element-access.html
+
+        * accessibility/AXObjectCache.cpp:
+        (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):
+           This changed exposed a case where an object was inlineElementContinuation, but not renderInlined,
+           which led to an assert.
+        (WebCore::AccessibilityRenderObject::updateAccessibilityRole):
+        (WebCore::AccessibilityRenderObject::childrenChanged):
+        * accessibility/AccessibilityRenderObject.h:
+
 2011-06-23  Dirk Schulze  <krit@webkit.org>
 
         Reviewed by Nikolas Zimmermann.
index 1c9b46a..a5d2a45 100644 (file)
@@ -82,6 +82,7 @@ bool AXObjectCache::gAccessibilityEnhancedUserInterfaceEnabled = false;
 
 AXObjectCache::AXObjectCache(const Document* doc)
     : m_notificationPostTimer(this, &AXObjectCache::notificationPostTimerFired)
+    , m_childrenUpdateTimer(this, &AXObjectCache::childrenUpdateTimerFired)
 {
     m_document = const_cast<Document*>(doc);
 }
@@ -367,6 +368,7 @@ void AXObjectCache::remove(RenderObject* renderer)
     AXID axID = m_renderObjectMapping.get(renderer);
     remove(axID);
     m_renderObjectMapping.remove(renderer);
+    m_childrenToUpdate.remove(renderer);
 }
 
 void AXObjectCache::remove(Widget* view)
@@ -436,6 +438,24 @@ void AXObjectCache::contentChanged(RenderObject* renderer)
         object->contentChanged(); 
 }
 #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)
 {
@@ -443,12 +463,16 @@ void AXObjectCache::childrenChanged(RenderObject* renderer)
         return;
  
     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);
+    }
 }
     
 void AXObjectCache::notificationPostTimerFired(Timer<AXObjectCache>*)
index 95cc7f3..8816f39 100644 (file)
@@ -166,7 +166,11 @@ private:
     Timer<AXObjectCache> m_notificationPostTimer;
     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*);
     
     AXID getAXID(AccessibilityObject*);
index e0c03c3..e8cfe8b 100644 (file)
@@ -68,13 +68,13 @@ void AccessibilityMenuList::addChildren()
     list->addChildren();
 }
 
-void AccessibilityMenuList::childrenChanged()
+void AccessibilityMenuList::childrenChanged(ChildrenChangeOptions options)
 {
     if (m_children.isEmpty())
         return;
 
     ASSERT(m_children.size() == 1);
-    m_children[0]->childrenChanged();
+    m_children[0]->childrenChanged(options);
 }
 
 bool AccessibilityMenuList::isCollapsed() const
index cdb3ac2..8d16c8f 100644 (file)
@@ -51,7 +51,7 @@ private:
     virtual bool canSetFocusAttribute() const { return true; }
 
     virtual void addChildren();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
 };
 
 } // namespace WebCore
index 9e2ed6b..d3b8342 100644 (file)
@@ -105,7 +105,7 @@ void AccessibilityMenuListPopup::addChildren()
     }
 }
 
-void AccessibilityMenuListPopup::childrenChanged()
+void AccessibilityMenuListPopup::childrenChanged(ChildrenChangeOptions)
 {
     for (size_t i = m_children.size(); i > 0 ; --i) {
         AccessibilityObject* child = m_children[i - 1].get();
index 88fbf7c..005b9d3 100644 (file)
@@ -56,7 +56,7 @@ private:
     virtual AccessibilityObject* parentObject() const;
     virtual bool press() const;
     virtual void addChildren();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
 
     AccessibilityMenuListOption* menuListOptionAccessibilityObject(HTMLElement*) const;
 
index e003153..0d5fd22 100644 (file)
@@ -464,7 +464,8 @@ public:
     virtual void increment() { }
     virtual void decrement() { }
 
-    virtual void childrenChanged() { }
+    enum ChildrenChangeOptions { DoNotCreateParentObjects, CreateParentObjects };
+    virtual void childrenChanged(ChildrenChangeOptions) { }
     virtual void contentChanged() { }
     virtual const AccessibilityChildrenVector& children() { return m_children; }
     virtual void addChildren() { }
index 4d4d7c0..6ccd931 100644 (file)
@@ -222,7 +222,7 @@ AccessibilityObject* AccessibilityRenderObject::lastChild() const
 
 static inline RenderInline* startOfContinuations(RenderObject* r)
 {
-    if (r->isInlineElementContinuation())
+    if (r->isInlineElementContinuation() && r->node()->renderer() && r->isRenderInline())
         return toRenderInline(r->node()->renderer());
 
     // Blocks with a previous continuation always have a next continuation
@@ -3030,7 +3030,7 @@ void AccessibilityRenderObject::updateAccessibilityRole()
     
     // The AX hierarchy only needs to be updated if the ignored status of an element has changed.
     if (ignoredStatus != accessibilityIsIgnored())
-        childrenChanged();
+        childrenChanged(DoNotCreateParentObjects);
 }
     
 AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()
@@ -3307,7 +3307,7 @@ void AccessibilityRenderObject::contentChanged()
     }
 }
     
-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.
     if (!m_renderer)
@@ -3319,7 +3319,7 @@ void AccessibilityRenderObject::childrenChanged()
     // called during render layouts, minimal work should be done. 
     // 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;
         
index d136b8a..2a45fd2 100644 (file)
@@ -207,7 +207,7 @@ public:
     virtual void decrement();
     
     virtual void detach();
-    virtual void childrenChanged();
+    virtual void childrenChanged(ChildrenChangeOptions);
     virtual void contentChanged();
     virtual void addChildren();
     virtual bool canHaveChildren() const;