⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284769 in webkit


Ignore:
Timestamp:
Oct 24, 2021, 3:28:37 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

AX: AccessibilityObject::m_haveChildren and AXCoreObject::hasChildren() are misleadingly named
​https://bugs.webkit.org/show_bug.cgi?id=232130

Patch by Tyler Wilcock <Tyler Wilcock> on 2021-10-24
Reviewed by Chris Fleizach.

The names of AccessibilityObject::m_haveChildren and AXCoreObject::hasChildren()
imply that the given object has one or more children. However, what these
really indicate is whether the object has tried to initialize its children.
Both m_haveChildren and hasChildren() can be true for objects that have no children,
which is confusing.

This patch:

  • Renames m_haveChildren to m_childrenInitialized and hasChildren() to childrenInitialized().
  • Removes AXPropertyName::HasChildren rather than renaming it because isolated object children are always initialized.
  • Fixes a bug in AccessibilityRenderObject::updateRoleAfterChildrenCreation caused by the poor names (we intended to change the role if there were no children, not if !hasChildren()).
  • accessibility/AccessibilityARIAGrid.cpp:

(WebCore::AccessibilityARIAGrid::addChildren):

  • accessibility/AccessibilityListBox.cpp:

(WebCore::AccessibilityListBox::addChildren):
(WebCore::AccessibilityListBox::selectedChildren):
(WebCore::AccessibilityListBox::visibleChildren):

  • accessibility/AccessibilityMenuList.cpp:

(WebCore::AccessibilityMenuList::addChildren):

  • accessibility/AccessibilityMenuListPopup.cpp:

(WebCore::AccessibilityMenuListPopup::addChildren):
(WebCore::AccessibilityMenuListPopup::childrenChanged):

  • accessibility/AccessibilityNodeObject.cpp:

(WebCore::AccessibilityNodeObject::addChildren):

  • accessibility/AccessibilityObject.cpp:

(WebCore::AccessibilityObject::updateChildrenIfNecessary):
(WebCore::AccessibilityObject::clearChildren):

  • accessibility/AccessibilityObject.h:
  • accessibility/AccessibilityObjectInterface.h:

(WebCore::AXCoreObject::isDescendantOfObject const):

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::addCanvasChildren):
(WebCore::AccessibilityRenderObject::updateRoleAfterChildrenCreation):
(WebCore::AccessibilityRenderObject::addChildren):
(WebCore::AccessibilityRenderObject::ariaListboxVisibleChildren):

  • accessibility/AccessibilityScrollView.cpp:

(WebCore::AccessibilityScrollView::addChildren):

  • accessibility/AccessibilitySlider.cpp:

(WebCore::AccessibilitySlider::addChildren):

  • accessibility/AccessibilitySpinButton.cpp:

(WebCore::AccessibilitySpinButton::incrementButton):
(WebCore::AccessibilitySpinButton::decrementButton):
(WebCore::AccessibilitySpinButton::addChildren):

  • accessibility/AccessibilityTable.cpp:

(WebCore::AccessibilityTable::addChildren):

  • accessibility/AccessibilityTableColumn.cpp:

(WebCore::AccessibilityTableColumn::addChildren):

  • accessibility/AccessibilityTableHeaderContainer.cpp:

(WebCore::AccessibilityTableHeaderContainer::addChildren):

  • accessibility/isolatedtree/AXIsolatedObject.cpp:

Stop setting AXPropertyName::HasChildren because it no longer exists.
(WebCore::AXIsolatedObject::initializeAttributeData):

  • accessibility/isolatedtree/AXIsolatedObject.h:
  • accessibility/isolatedtree/AXIsolatedTree.h:

Delete AXPropertyName::HasChildren.

Location:
trunk/Source/WebCore
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r284767 r284769  
     12021-10-24  Tyler Wilcock  <tyler_w@apple.com>
     2
     3        AX: AccessibilityObject::m_haveChildren and AXCoreObject::hasChildren() are misleadingly named
     4        https://bugs.webkit.org/show_bug.cgi?id=232130
     5
     6        Reviewed by Chris Fleizach.
     7
     8        The names of `AccessibilityObject::m_haveChildren` and `AXCoreObject::hasChildren()`
     9        imply that the given object has one or more children. However, what these
     10        really indicate is whether the object has tried to initialize its children.
     11        Both `m_haveChildren` and `hasChildren()` can be true for objects that have no children,
     12        which is confusing.
     13
     14        This patch:
     15
     16          - Renames `m_haveChildren` to `m_childrenInitialized` and
     17            `hasChildren()` to `childrenInitialized()`.
     18
     19          - Removes `AXPropertyName::HasChildren` rather than
     20            renaming it because isolated object children are always initialized.
     21
     22          - Fixes a bug in `AccessibilityRenderObject::updateRoleAfterChildrenCreation`
     23            caused by the poor names (we intended to change the role if there
     24            were no children, not if `!hasChildren()`).
     25
     26        * accessibility/AccessibilityARIAGrid.cpp:
     27        (WebCore::AccessibilityARIAGrid::addChildren):
     28        * accessibility/AccessibilityListBox.cpp:
     29        (WebCore::AccessibilityListBox::addChildren):
     30        (WebCore::AccessibilityListBox::selectedChildren):
     31        (WebCore::AccessibilityListBox::visibleChildren):
     32        * accessibility/AccessibilityMenuList.cpp:
     33        (WebCore::AccessibilityMenuList::addChildren):
     34        * accessibility/AccessibilityMenuListPopup.cpp:
     35        (WebCore::AccessibilityMenuListPopup::addChildren):
     36        (WebCore::AccessibilityMenuListPopup::childrenChanged):
     37        * accessibility/AccessibilityNodeObject.cpp:
     38        (WebCore::AccessibilityNodeObject::addChildren):
     39        * accessibility/AccessibilityObject.cpp:
     40        (WebCore::AccessibilityObject::updateChildrenIfNecessary):
     41        (WebCore::AccessibilityObject::clearChildren):
     42        * accessibility/AccessibilityObject.h:
     43        * accessibility/AccessibilityObjectInterface.h:
     44        (WebCore::AXCoreObject::isDescendantOfObject const):
     45        * accessibility/AccessibilityRenderObject.cpp:
     46        (WebCore::AccessibilityRenderObject::addCanvasChildren):
     47        (WebCore::AccessibilityRenderObject::updateRoleAfterChildrenCreation):
     48        (WebCore::AccessibilityRenderObject::addChildren):
     49        (WebCore::AccessibilityRenderObject::ariaListboxVisibleChildren):
     50        * accessibility/AccessibilityScrollView.cpp:
     51        (WebCore::AccessibilityScrollView::addChildren):
     52        * accessibility/AccessibilitySlider.cpp:
     53        (WebCore::AccessibilitySlider::addChildren):
     54        * accessibility/AccessibilitySpinButton.cpp:
     55        (WebCore::AccessibilitySpinButton::incrementButton):
     56        (WebCore::AccessibilitySpinButton::decrementButton):
     57        (WebCore::AccessibilitySpinButton::addChildren):
     58        * accessibility/AccessibilityTable.cpp:
     59        (WebCore::AccessibilityTable::addChildren):
     60        * accessibility/AccessibilityTableColumn.cpp:
     61        (WebCore::AccessibilityTableColumn::addChildren):
     62        * accessibility/AccessibilityTableHeaderContainer.cpp:
     63        (WebCore::AccessibilityTableHeaderContainer::addChildren):
     64        * accessibility/isolatedtree/AXIsolatedObject.cpp:
     65        Stop setting `AXPropertyName::HasChildren` because it no longer exists.
     66        (WebCore::AXIsolatedObject::initializeAttributeData):
     67        * accessibility/isolatedtree/AXIsolatedObject.h:
     68        * accessibility/isolatedtree/AXIsolatedTree.h:
     69        Delete `AXPropertyName::HasChildren`.
     70
    1712021-10-24  Fujii Hironori  <Hironori.Fujii@sony.com>
    272

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

    r284760 r284769  
    9595void AccessibilityARIAGrid::addChildren()
    9696{
    97     ASSERT(!m_haveChildren);
     97    ASSERT(!m_childrenInitialized);
    9898   
    9999    if (!isExposable()) {
    … …  
    102102    }
    103103   
    104     m_haveChildren = true;
     104    m_childrenInitialized = true;
    105105    if (!m_renderer)
    106106        return;

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

    r284760 r284769  
    7272        return;
    7373
    74     m_haveChildren = true;
     74    m_childrenInitialized = true;
    7575
    7676    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems())
    … …  
    106106    ASSERT(result.isEmpty());
    107107
    108     if (!hasChildren())
     108    if (!childrenInitialized())
    109109        addChildren();
    110        
     110
    111111    for (const auto& child : m_children) {
    112112        if (downcast<AccessibilityListBoxOption>(*child).isSelected())
    … …  
    119119    ASSERT(result.isEmpty());
    120120   
    121     if (!hasChildren())
     121    if (!childrenInitialized())
    122122        addChildren();
    123123   

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

    r284760 r284769  
    8282    }
    8383
    84     m_haveChildren = true;
     84    m_childrenInitialized = true;
    8585    addChild(list);
    8686    list->addChildren();

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

    r284760 r284769  
    9595        return;
    9696
    97     m_haveChildren = true;
     97    m_childrenInitialized = true;
    9898
    9999    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems()) {
    … …  
    117117   
    118118    m_children.clear();
    119     m_haveChildren = false;
     119    m_childrenInitialized = false;
    120120    addChildren();
    121121}

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

    r284266 r284769  
    343343    // If the need to add more children in addition to existing children arises,
    344344    // childrenChanged should have been called, leaving the object with no children.
    345     ASSERT(!m_haveChildren);
     345    ASSERT(!m_childrenInitialized);
    346346   
    347347    if (!m_node)
    348348        return;
    349349
    350     m_haveChildren = true;
     350    m_childrenInitialized = true;
    351351
    352352    // The only time we add children from the DOM tree to a node with a renderer is when it's a canvas.

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

    r284760 r284769  
    16871687void AccessibilityObject::updateChildrenIfNecessary()
    16881688{
    1689     if (!hasChildren()) {
     1689    if (!childrenInitialized()) {
    16901690        // Enable the cache in case we end up adding a lot of children, we don't want to recompute axIsIgnored each time.
    16911691        AXAttributeCacheEnabler enableCache(axObjectCache());
    … …  
    17011701   
    17021702    m_children.clear();
    1703     m_haveChildren = false;
     1703    m_childrenInitialized = false;
    17041704}
    17051705

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

    r284760 r284769  
    496496
    497497    bool canHaveChildren() const override { return true; }
    498     bool hasChildren() const override { return m_haveChildren; }
     498    bool childrenInitialized() const override { return m_childrenInitialized; }
    499499    void updateChildrenIfNecessary() override;
    500500    void setNeedsToUpdateChildren() override { }
    … …  
    812812protected: // FIXME: Make the data members private.
    813813    AccessibilityChildrenVector m_children;
    814     mutable bool m_haveChildren { false };
     814    mutable bool m_childrenInitialized { false };
    815815    AccessibilityRole m_role { AccessibilityRole::Unknown };
    816816private:

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

    r284760 r284769  
    12501250
    12511251    virtual bool canHaveChildren() const = 0;
    1252     virtual bool hasChildren() const = 0;
     1252    virtual bool childrenInitialized() const = 0;
    12531253    virtual void updateChildrenIfNecessary() = 0;
    12541254    virtual void setNeedsToUpdateChildren() = 0;
    … …  
    16261626inline bool AXCoreObject::isDescendantOfObject(const AXCoreObject* axObject) const
    16271627{
    1628     return axObject && axObject->hasChildren()
     1628    return axObject && axObject->childrenInitialized()
    16291629        && Accessibility::findAncestor<AXCoreObject>(*this, false, [axObject] (const AXCoreObject& object) {
    16301630            return &object == axObject;

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

    r284760 r284769  
    33863386
    33873387    // If it's a canvas, it won't have rendered children, but it might have accessible fallback content.
    3388     // Clear m_haveChildren because AccessibilityNodeObject::addChildren will expect it to be false.
     3388    // Clear m_childrenInitialized because AccessibilityNodeObject::addChildren will expect it to be false.
    33893389    ASSERT(!m_children.size());
    3390     m_haveChildren = false;
     3390    m_childrenInitialized = false;
    33913391    AccessibilityNodeObject::addChildren();
    33923392}
    … …  
    34913491            m_role = AccessibilityRole::Group;
    34923492    }
    3493     if (role == AccessibilityRole::SVGRoot && !hasChildren())
     3493    if (role == AccessibilityRole::SVGRoot && !children().size())
    34943494        m_role = AccessibilityRole::Image;
    34953495}
    … …  
    34993499    // If the need to add more children in addition to existing children arises,
    35003500    // childrenChanged should have been called, leaving the object with no children.
    3501     ASSERT(!m_haveChildren);
    3502 
    3503     m_haveChildren = true;
     3501    ASSERT(!m_childrenInitialized);
     3502
     3503    m_childrenInitialized = true;
    35043504   
    35053505    if (!canHaveChildren())
    … …  
    36903690void AccessibilityRenderObject::ariaListboxVisibleChildren(AccessibilityChildrenVector& result)     
    36913691{
    3692     if (!hasChildren())
     3692    if (!childrenInitialized())
    36933693        addChildren();
    36943694   

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

    r284760 r284769  
    186186void AccessibilityScrollView::addChildren()
    187187{
    188     ASSERT(!m_haveChildren);
    189     m_haveChildren = true;
     188    ASSERT(!m_childrenInitialized);
     189    m_childrenInitialized = true;
    190190   
    191191    addChild(webAreaObject());

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

    r284760 r284769  
    8787void AccessibilitySlider::addChildren()
    8888{
    89     ASSERT(!m_haveChildren);
     89    ASSERT(!m_childrenInitialized);
    9090   
    91     m_haveChildren = true;
     91    m_childrenInitialized = true;
    9292
    9393    AXObjectCache* cache = m_renderer->document().axObjectCache();

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

    r284760 r284769  
    4646AXCoreObject* AccessibilitySpinButton::incrementButton()
    4747{
    48     if (!m_haveChildren)
     48    if (!m_childrenInitialized)
    4949        addChildren();
    50     if (!m_haveChildren)
     50    if (!m_childrenInitialized)
    5151        return nullptr;
    5252
    … …  
    5858AXCoreObject* AccessibilitySpinButton::decrementButton()
    5959{
    60     if (!m_haveChildren)
     60    if (!m_childrenInitialized)
    6161        addChildren();
    62     if (!m_haveChildren)
     62    if (!m_childrenInitialized)
    6363        return nullptr;
    6464   
    … …  
    8787        return;
    8888   
    89     m_haveChildren = true;
     89    m_childrenInitialized = true;
    9090   
    9191    auto& incrementor = downcast<AccessibilitySpinButtonPart>(*cache->create(AccessibilityRole::SpinButtonPart));

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

    r284760 r284769  
    382382    }
    383383   
    384     ASSERT(!m_haveChildren);
    385    
    386     m_haveChildren = true;
     384    ASSERT(!m_childrenInitialized);
     385   
     386    m_childrenInitialized = true;
    387387    if (!is<RenderTable>(renderer()))
    388388        return;

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

    r284760 r284769  
    182182void AccessibilityTableColumn::addChildren()
    183183{
    184     ASSERT(!m_haveChildren);
    185    
    186     m_haveChildren = true;
     184    ASSERT(!m_childrenInitialized);
     185   
     186    m_childrenInitialized = true;
    187187    if (!is<AccessibilityTable>(m_parent))
    188188        return;

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

    r284760 r284769  
    6464void AccessibilityTableHeaderContainer::addChildren()
    6565{
    66     ASSERT(!m_haveChildren);
     66    ASSERT(!m_childrenInitialized);
    6767   
    68     m_haveChildren = true;
     68    m_childrenInitialized = true;
    6969    if (!is<AccessibilityTable>(m_parent))
    7070        return;

  • trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp

    r284760 r284769  
    164164    setProperty(AXPropertyName::EstimatedLoadingProgress, object.estimatedLoadingProgress());
    165165    setProperty(AXPropertyName::SupportsARIAOwns, object.supportsARIAOwns());
    166     setProperty(AXPropertyName::HasChildren, object.hasChildren());
    167166    setProperty(AXPropertyName::HasPopup, object.hasPopup());
    168167    setProperty(AXPropertyName::PopupValue, object.popupValue().isolatedCopy());

  • trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h

    r284760 r284769  
    611611    void insertChild(AXCoreObject*, unsigned, DescendIfIgnored = DescendIfIgnored::Yes) override;
    612612    bool canHaveChildren() const override;
    613     bool hasChildren() const override { return boolAttributeValue(AXPropertyName::HasChildren); }
     613    bool childrenInitialized() const override { return true; }
    614614    void setNeedsToUpdateChildren() override;
    615615    void setNeedsToUpdateSubtree() override;

  • trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

    r284529 r284769  
    119119    HasApplePDFAnnotationAttribute,
    120120    HasBoldFont,
    121     HasChildren,
    122121    HasHighlighting,
    123122    HasItalicFont,
Note: See TracChangeset for help on using the changeset viewer.