Changeset 263673 in webkit


Ignore:
Timestamp:
Jun 29, 2020 11:54:16 AM (4 years ago)
Author:
Chris Fleizach
Message:

AX: aria-modal nodes wrapped in aria-hidden are not honored
​https://bugs.webkit.org/show_bug.cgi?id=212849
<rdar://problem/64047019>

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: accessibility/aria-modal-in-aria-hidden.html

If aria-modal was wrapped inside aria-hidden, we were still processing that as the modal node.
Fixing that uncovered a host of very finicky issues related to aria-modal.

  1. We were processing modal status immediately instead of after a delay, so visibility requirements were not correct.
  2. In handleModalChange: We were processing multiple modal nodes perhaps incorrectly (the spec doesn't account for multiple modal nodes).
    • had to update a test to turn off modal status before adding a new modal node
  3. Changed the modal node to a WeakPtr
  4. In isNodeAriaVisible: We stopped processing for visibile with aria-hidden as soon as we hit a renderable block, but that means it won't account for nodes higher in the tree with aria-hidden.
  5. In handleAttributeChange: if aria-hidden changes, we should update modal status if needed.
  6. In focusModalNodeTimerFired: we need to verify the element is still live, otherwise it can lead to a crash.
  • accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::AXObjectCache):
(WebCore::AXObjectCache::findModalNodes):
(WebCore::AXObjectCache::currentModalNode):
(WebCore::AXObjectCache::modalNode):
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::deferModalChange):
(WebCore::AXObjectCache::focusModalNodeTimerFired):
(WebCore::AXObjectCache::handleAttributeChange):
(WebCore::AXObjectCache::handleModalChange):
(WebCore::AXObjectCache::prepareForDocumentDestruction):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
(WebCore::isNodeAriaVisible):

  • accessibility/AXObjectCache.h:

(WebCore::AXObjectCache::handleModalChange):
(WebCore::AXObjectCache::deferModalChange):

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::firstChild const):
(WebCore::AccessibilityRenderObject::lastChild const):

LayoutTests:

  • accessibility/aria-hidden-negates-no-visibility-expected.txt:
  • accessibility/aria-hidden-negates-no-visibility.html:
  • accessibility/aria-modal-in-aria-hidden-expected.txt: Added.
  • accessibility/aria-modal-in-aria-hidden.html: Added.
  • accessibility/aria-modal.html:
  • accessibility/mac/aria-modal-auto-focus.html:
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r263672 r263673  
     12020-06-29  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: aria-modal nodes wrapped in aria-hidden are not honored
     4        https://bugs.webkit.org/show_bug.cgi?id=212849
     5        <rdar://problem/64047019>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        * accessibility/aria-hidden-negates-no-visibility-expected.txt:
     10        * accessibility/aria-hidden-negates-no-visibility.html:
     11        * accessibility/aria-modal-in-aria-hidden-expected.txt: Added.
     12        * accessibility/aria-modal-in-aria-hidden.html: Added.
     13        * accessibility/aria-modal.html:
     14        * accessibility/mac/aria-modal-auto-focus.html:
     15
    1162020-06-29  Karl Rackler  <rackler@apple.com>
    217

  • trunk/LayoutTests/accessibility/aria-hidden-negates-no-visibility-expected.txt

    r198895 r263673  
    11heading1.2
    2 
    32
    43
    … …  
    1918PASS parent.childAtIndex(2).isEqual(heading4) is true
    2019Textfield Title: AXTitle:
    21 PASS video.childrenCount is 0
     20PASS !video || video.childrenCount == 0 is true
    2221PASS successfullyParsed is true
    2322

  • trunk/LayoutTests/accessibility/aria-hidden-negates-no-visibility.html

    r198895 r263673  
    1717<input type="text" aria-labelledby="hiddenDiv" id="textFieldWithHiddenLabeller">
    1818
     19<div>
    1920<div aria-hidden="false">
    20 <video id="video">
     21<video hidden id="video">
    2122Hidden content
    2223</video>
     24</div>
    2325</div>
    2426
    … …  
    6466        // aria-hidden="false" need to be on each parent, including rendered parents.
    6567        var video = accessibilityController.accessibleElementById("video");
    66         shouldBe("video.childrenCount", "0");
     68        shouldBeTrue("!video || video.childrenCount == 0");
    6769    }
    6870

  • trunk/LayoutTests/accessibility/aria-modal.html

    r191931 r263673  
    2222
    2323    description("This tests that aria-modal on dialog makes other elements inert.");
    24 
     24    jsTestIsAsync = true;
    2525    if (window.accessibilityController) {
    2626        // Background should be unaccessible after loading, since the
    … …  
    4848        shouldBeTrue("backgroundAccessible()");
    4949        document.getElementById("box").setAttribute("aria-hidden", "false");
    50         shouldBeFalse("backgroundAccessible()");
     50        setTimeout(function() {
     51            shouldBeFalse("backgroundAccessible()");
    5152       
    52         // Test modal dialog is removed from DOM tree.
    53         var dialog = document.getElementById("box");
    54         dialog.parentNode.removeChild(dialog);
    55         shouldBeTrue("backgroundAccessible()");
     53            // Test modal dialog is removed from DOM tree.
     54            var dialog = document.getElementById("box");
     55            dialog.parentNode.removeChild(dialog);
     56            shouldBeTrue("backgroundAccessible()");
     57            finishJSTest();
     58         }, 0);
    5659    }
    5760   

  • trunk/LayoutTests/accessibility/mac/aria-modal-auto-focus.html

    r228279 r263673  
    5050           
    5151            // 2. Click the new button, dialog2 shows and focus should move to the close button.
     52            // Set aria-modal to false on the previous modal object (we shouldn't have two modals in play).
    5253            document.getElementById("new").click();
     54            document.getElementById("box").ariaModal = false;
    5355            setTimeout(function(){
    5456                closeBtn = accessibilityController.accessibleElementById("close");
    … …  
    5658                 
    5759                // 3. Click the close button, dialog2 closes and focus should go back to the
    58                 // first focusable child of dialog1.
     60                // first focusable child of dialog1, which we now need to add aria-modal back to.
    5961                document.getElementById("close").click();
     62                document.getElementById("box").ariaModal = true;
    6063                setTimeout(function(){
    6164                    okBtn = accessibilityController.accessibleElementById("ok");

  • trunk/Source/WebCore/ChangeLog

    r263671 r263673  
     12020-06-29  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: aria-modal nodes wrapped in aria-hidden are not honored
     4        https://bugs.webkit.org/show_bug.cgi?id=212849
     5        <rdar://problem/64047019>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Test: accessibility/aria-modal-in-aria-hidden.html
     10
     11        If aria-modal was wrapped inside aria-hidden, we were still processing that as the modal node.
     12        Fixing that uncovered a host of very finicky issues related to aria-modal.
     13        1. We were processing modal status immediately instead of after a delay, so visibility requirements were not correct.
     14        2. In handleModalChange: We were processing multiple modal nodes perhaps incorrectly (the spec doesn't account for multiple modal nodes).
     15             - had to update a test to turn off modal status before adding a new modal node
     16        3. Changed the modal node to a WeakPtr
     17        4. In isNodeAriaVisible: We stopped processing for visibile with aria-hidden as soon as we hit a renderable block, but that means it won't account
     18           for nodes higher in the tree with aria-hidden.
     19        5. In handleAttributeChange: if aria-hidden changes, we should update modal status if needed.
     20        6. In focusModalNodeTimerFired: we need to verify the element is still live, otherwise it can lead to a crash.
     21
     22        * accessibility/AXObjectCache.cpp:
     23        (WebCore::AXObjectCache::AXObjectCache):
     24        (WebCore::AXObjectCache::findModalNodes):
     25        (WebCore::AXObjectCache::currentModalNode):
     26        (WebCore::AXObjectCache::modalNode):
     27        (WebCore::AXObjectCache::remove):
     28        (WebCore::AXObjectCache::deferModalChange):
     29        (WebCore::AXObjectCache::focusModalNodeTimerFired):
     30        (WebCore::AXObjectCache::handleAttributeChange):
     31        (WebCore::AXObjectCache::handleModalChange):
     32        (WebCore::AXObjectCache::prepareForDocumentDestruction):
     33        (WebCore::AXObjectCache::performDeferredCacheUpdate):
     34        (WebCore::isNodeAriaVisible):
     35        * accessibility/AXObjectCache.h:
     36        (WebCore::AXObjectCache::handleModalChange):
     37        (WebCore::AXObjectCache::deferModalChange):
     38        * accessibility/AccessibilityRenderObject.cpp:
     39        (WebCore::AccessibilityRenderObject::firstChild const):
     40        (WebCore::AccessibilityRenderObject::lastChild const):
     41
    1422020-06-29  Youenn Fablet  <youenn@apple.com>
    243

  • trunk/Source/WebCore/accessibility/AXObjectCache.cpp

    r263571 r263673  
    221221    , m_liveRegionChangedPostTimer(*this, &AXObjectCache::liveRegionChangedNotificationPostTimerFired)
    222222    , m_focusModalNodeTimer(*this, &AXObjectCache::focusModalNodeTimerFired)
    223     , m_currentModalNode(nullptr)
     223    , m_currentModalElement(nullptr)
    224224    , m_performCacheUpdateTimer(*this, &AXObjectCache::performCacheUpdateTimerFired)
    225225{
    … …  
    253253            continue;
    254254
    255         m_modalNodesSet.add(element);
     255        m_modalElementsSet.add(element);
    256256    }
    257257
    … …  
    259259}
    260260
    261 Node* AXObjectCache::currentModalNode()
     261Element* AXObjectCache::currentModalNode()
    262262{
    263263    // There might be multiple nodes with aria-modal=true set.
    264264    // We use this function to pick the one we want.
    265     m_currentModalNode = nullptr;
    266     if (m_modalNodesSet.isEmpty())
     265    m_currentModalElement = nullptr;
     266    if (m_modalElementsSet.isEmpty())
    267267        return nullptr;
    268268
    … …  
    270270    // If not, we want to pick the last visible dialog in the DOM.
    271271    RefPtr<Element> focusedElement = document().focusedElement();
    272     RefPtr<Node> lastVisible;
    273     for (auto& node : m_modalNodesSet) {
    274         if (isNodeVisible(node)) {
    275             if (focusedElement && focusedElement->isDescendantOf(node)) {
    276                 m_currentModalNode = node;
     272    RefPtr<Element> lastVisible;
     273    for (auto& element : m_modalElementsSet) {
     274        if (isNodeVisible(element)) {
     275            if (focusedElement && focusedElement->isDescendantOf(element)) {
     276                m_currentModalElement = makeWeakPtr(element);
    277277                break;
    278278            }
    279279
    280             lastVisible = node;
     280            lastVisible = element;
    281281        }
    282282    }
    283283
    284     if (!m_currentModalNode)
    285         m_currentModalNode = lastVisible.get();
    286 
    287     return m_currentModalNode;
     284    if (!m_currentModalElement)
     285        m_currentModalElement = makeWeakPtr(lastVisible.get());
     286
     287    return m_currentModalElement.get();
    288288}
    289289
    … …  
    307307}
    308308
     309// This function returns the valid aria modal node.
    309310Node* AXObjectCache::modalNode()
    310311{
    311     // This function returns the valid aria modal node.
    312     if (!m_modalNodesInitialized) {
     312    if (!m_modalNodesInitialized)
    313313        findModalNodes();
    314         return currentModalNode();
    315     }
    316 
    317     if (m_modalNodesSet.isEmpty())
     314
     315    if (m_modalElementsSet.isEmpty())
    318316        return nullptr;
    319317
    320318    // Check the cached current valid aria modal node first.
    321319    // Usually when one dialog sets aria-modal=true, that dialog is the one we want.
    322     if (isNodeVisible(m_currentModalNode))
    323         return m_currentModalNode;
    324 
    325     // Recompute the valid aria modal node when m_currentModalNode is null or hidden.
     320    if (isNodeVisible(m_currentModalElement.get()))
     321        return m_currentModalElement.get();
     322
     323    // Recompute the valid aria modal node when m_currentModalElement is null or hidden.
    326324    return currentModalNode();
    327325}
    … …  
    875873        m_deferredTextFormControlValue.remove(downcast<Element>(&node));
    876874        m_deferredAttributeChange.remove(downcast<Element>(&node));
     875        m_modalElementsSet.remove(downcast<Element>(&node));
    877876    }
    878877    m_deferredChildrenChangedNodeList.remove(&node);
    … …  
    891890
    892891    remove(m_nodeObjectMapping.take(&node));
    893 
    894     if (m_currentModalNode == &node)
    895         m_currentModalNode = nullptr;
    896     m_modalNodesSet.remove(&node);
    897 
    898892    remove(node.renderer());
    899893}
    … …  
    11881182    } else
    11891183        handleFocusedUIElementChanged(oldNode, newNode);
     1184}
     1185
     1186void AXObjectCache::deferModalChange(Element* element)
     1187{
     1188    m_deferredModalChangedList.add(element);
     1189    if (!m_performCacheUpdateTimer.isActive())
     1190        m_performCacheUpdateTimer.startOneShot(0_s);
    11901191}
    11911192   
    … …  
    15871588void AXObjectCache::focusModalNodeTimerFired()
    15881589{
    1589     if (!m_currentModalNode)
     1590    if (!m_document.hasLivingRenderTree())
     1591        return;
     1592
     1593    Ref<Document> protectedDocument(m_document);
     1594    if (!nodeAndRendererAreValid(m_currentModalElement.get()) || !isNodeVisible(m_currentModalElement.get()))
    15901595        return;
    15911596   
    15921597    // Don't set focus if we are already focusing onto some element within
    15931598    // the dialog.
    1594     if (m_currentModalNode->contains(document().focusedElement()))
    1595         return;
    1596    
    1597     if (AccessibilityObject* currentModalNodeObject = getOrCreate(m_currentModalNode)) {
     1599    if (m_currentModalElement->contains(document().focusedElement()))
     1600        return;
     1601   
     1602    if (AccessibilityObject* currentModalNodeObject = getOrCreate(m_currentModalElement.get())) {
    15981603        if (AccessibilityObject* focusable = firstFocusableChild(currentModalNodeObject))
    15991604            focusable->setFocused(true);
    … …  
    16931698    else if (attrName == aria_expandedAttr)
    16941699        handleAriaExpandedChange(element);
    1695     else if (attrName == aria_hiddenAttr)
     1700    else if (attrName == aria_hiddenAttr) {
    16961701        childrenChanged(element->parentNode(), element);
     1702        if (m_currentModalElement && m_currentModalElement->isDescendantOf(element)) {
     1703            m_modalNodesInitialized = false;
     1704            deferModalChange(m_currentModalElement.get());
     1705        }
     1706    }
    16971707    else if (attrName == aria_invalidAttr)
    16981708        postNotification(element, AXObjectCache::AXInvalidStatusChanged);
    16991709    else if (attrName == aria_modalAttr)
    1700         handleModalChange(element);
     1710        deferModalChange(element);
    17011711    else if (attrName == aria_currentAttr)
    17021712        postNotification(element, AXObjectCache::AXCurrentChanged);
    … …  
    17131723}
    17141724
    1715 void AXObjectCache::handleModalChange(Node* node)
    1716 {
    1717     if (!is<Element>(node))
    1718         return;
    1719 
    1720     if (!nodeHasRole(node, "dialog") && !nodeHasRole(node, "alertdialog"))
     1725void AXObjectCache::handleModalChange(Element& element)
     1726{
     1727    if (!nodeHasRole(&element, "dialog") && !nodeHasRole(&element, "alertdialog"))
    17211728        return;
    17221729
    … …  
    17261733        findModalNodes();
    17271734
    1728     if (equalLettersIgnoringASCIICase(downcast<Element>(*node).attributeWithoutSynchronization(aria_modalAttr), "true")) {
    1729         // Add the newly modified node to the modal nodes set, and set it to be the current valid aria modal node.
     1735    if (equalLettersIgnoringASCIICase(element.attributeWithoutSynchronization(aria_modalAttr), "true")) {
     1736        // Add the newly modified node to the modal nodes set.
    17301737        // We will recompute the current valid aria modal node in modalNode() when this node is not visible.
    1731         m_modalNodesSet.add(node);
    1732         m_currentModalNode = node;
     1738        m_modalElementsSet.add(&element);
    17331739    } else {
    1734         // Remove the node from the modal nodes set. There might be other visible modal nodes, so we recompute here.
    1735         m_modalNodesSet.remove(node);
    1736         currentModalNode();
    1737     }
    1738 
    1739     if (m_currentModalNode)
     1740        // Remove the node from the modal nodes set.
     1741        m_modalElementsSet.remove(&element);
     1742    }
     1743
     1744    // Find new active modal node.
     1745    currentModalNode();
     1746
     1747    if (m_currentModalElement)
    17401748        focusModalNode();
    17411749
    … …  
    30363044    HashSet<Node*> nodesToRemove;
    30373045    filterListForRemoval(m_textMarkerNodes, document, nodesToRemove);
    3038     filterListForRemoval(m_modalNodesSet, document, nodesToRemove);
     3046    filterListForRemoval(m_modalElementsSet, document, nodesToRemove);
    30393047    filterListForRemoval(m_deferredTextChangedList, document, nodesToRemove);
    30403048    filterListForRemoval(m_deferredChildrenChangedNodeList, document, nodesToRemove);
    … …  
    30423050    filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
    30433051    filterVectorPairForRemoval(m_deferredFocusedNodeChange, document, nodesToRemove);
    3044 
     3052   
    30453053    for (auto* node : nodesToRemove)
    30463054        remove(*node);
    … …  
    31103118        handleFocusedUIElementChanged(deferredFocusedChangeContext.first, deferredFocusedChangeContext.second);
    31113119    m_deferredFocusedNodeChange.clear();
     3120
     3121    for (auto& deferredModalChangedElement : m_deferredModalChangedList)
     3122        handleModalChange(deferredModalChangedElement);
     3123    m_deferredModalChangedList.clear();
    31123124
    31133125    platformPerformDeferredCacheUpdate();
    … …  
    33033315            if (equalLettersIgnoringASCIICase(ariaHiddenValue, "true"))
    33043316                return false;
    3305            
     3317
     3318            // We should break early when it gets to the body.
     3319            if (testNode->hasTagName(bodyTag))
     3320                break;
     3321
    33063322            bool ariaHiddenFalse = equalLettersIgnoringASCIICase(ariaHiddenValue, "false");
    33073323            if (!testNode->renderer() && !ariaHiddenFalse)
    … …  
    33093325            if (!ariaHiddenFalsePresent && ariaHiddenFalse)
    33103326                ariaHiddenFalsePresent = true;
    3311             // We should break early when it gets to a rendered object.
    3312             if (testNode->renderer())
    3313                 break;
    33143327        }
    33153328    }

  • trunk/Source/WebCore/accessibility/AXObjectCache.h

    r261749 r263673  
    191191
    192192    void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
     193    void deferModalChange(Element*);
    193194    void handleScrolledToAnchor(const Node* anchorNode);
    194195    void handleScrollbarUpdate(ScrollView*);
    … …  
    463464    // aria-modal related
    464465    void findModalNodes();
    465     Node* currentModalNode();
     466    Element* currentModalNode();
    466467    bool isNodeVisible(Node*) const;
    467     void handleModalChange(Node*);
    468 
     468    void handleModalChange(Element&);
     469   
    469470    Document& m_document;
    470471    const Optional<PageIdentifier> m_pageID; // constant for object's lifetime.
    … …  
    491492
    492493    Timer m_focusModalNodeTimer;
    493     Node* m_currentModalNode;
    494     ListHashSet<Node*> m_modalNodesSet;
     494    WeakPtr<Element> m_currentModalElement;
     495    // Multiple aria-modals behavior is undefined by spec. We keep them sorted based on DOM order here.
     496    // If that changes to require only one aria-modal we could change this to a WeakHashSet, or discard the set completely.
     497    ListHashSet<Element*> m_modalElementsSet;
    495498    bool m_modalNodesInitialized { false };
    496499
    … …  
    503506    ListHashSet<RefPtr<AXCoreObject>> m_deferredChildrenChangedList;
    504507    ListHashSet<Node*> m_deferredChildrenChangedNodeList;
     508    WeakHashSet<Element> m_deferredModalChangedList;
    505509    HashMap<Element*, String> m_deferredTextFormControlValue;
    506510    HashMap<Element*, QualifiedName> m_deferredAttributeChange;
    … …  
    574578inline void AXObjectCache::handleActiveDescendantChanged(Node*) { }
    575579inline void AXObjectCache::handleAriaExpandedChange(Node*) { }
    576 inline void AXObjectCache::handleModalChange(Node*) { }
     580inline void AXObjectCache::handleModalChange(Element*) { }
     581inline void AXObjectCache::deferModalChange(Element*) { }
    577582inline void AXObjectCache::handleAriaRoleChanged(Node*) { }
    578583inline void AXObjectCache::deferAttributeChangeIfNeeded(const QualifiedName&, Element*) { }

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r263096 r263673  
    229229        return AccessibilityNodeObject::firstChild();
    230230
    231     return axObjectCache()->getOrCreate(firstChild);
     231    auto objectCache = axObjectCache();
     232    return objectCache ? objectCache->getOrCreate(firstChild) : nullptr;
    232233}
    233234
    … …  
    242243        return AccessibilityNodeObject::lastChild();
    243244
    244     return axObjectCache()->getOrCreate(lastChild);
     245    auto objectCache = axObjectCache();
     246    return objectCache ? objectCache->getOrCreate(lastChild) : nullptr;
    245247}
    246248
Note: See TracChangeset for help on using the changeset viewer.