Attachment 466439 Details for Bug 256950 – Patch

[patch] Patch

bug-256950-20230520113124.patch (text/plain), 32.92 KB, created by Tyler Wilcock on 2023-05-20 11:31:25 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tyler Wilcock

Created: 2023-05-20 11:31:25 PDT

Size: 32.92 KB

patch

obsolete

>From ddb3bf557a9eae2a49027091c4e4c245aa93c0c8 Mon Sep 17 00:00:00 2001
>From: Tyler Wilcock <tyler_w@apple.com>
>Date: Wed, 17 May 2023 23:23:53 -0700
>Subject: [PATCH] AX: display:contents lists don't return the correct subrole
> https://bugs.webkit.org/show_bug.cgi?id=256950 rdar://problem/109498423
>
>Reviewed by NOBODY (OOPS!).
>
>With this patch, any list-type element (ol, ul, dl, role="list", etc)
>with display:contents will now properly be created as an
>AccessibilityList instance rather than an AccessibilityNodeObject.
>
>The significant effects of this are:
>  1. The correct subrole is now computed for these elements, which is
>     important because some ATs do change their behavior based on the
>     various list subroles.
>  2. These elements now use the AccessibilityList::determineAccessibilityRole()
>     "is this an AX list or layout-only list" heuristics.
>
>In support of #2, some tests had to be modified to have rendered list items in
>order to still be considered lists.
>
>We implement this by allowing the creation of AccessibilityList with only a node
>instead of requiring a renderer.
>
>The other, more significant change introduced in this patch is that many
>AccessibilityRenderObject methods are now changed to fallback to the
>corresponding AccessibilityNodeObject implementation when there is no
>renderer rather than returning early.
>
>This is required because we have many subclasses of AccessibilityRenderObject
>that also need to support display:contents (AccessibilityList is one of these).
>Making this change weakens our definition of AccessibilityRenderObject from
>"should have a renderer" to "should have a renderer or a node".
>
>* LayoutTests/platform/glib/accessibility/aria-visible-element-roles-expected.txt:
>* LayoutTests/platform/glib/accessibility/display-contents-element-roles-expected.txt:
>* LayoutTests/platform/ios/accessibility/display-contents-element-roles-expected.txt:
>* LayoutTests/platform/mac-wk1/accessibility/aria-visible-element-roles-expected.txt:
>* LayoutTests/accessibility/aria-visible-element-roles.html:
>* LayoutTests/accessibility/display-contents-element-roles-expected.txt:
>* LayoutTests/accessibility/display-contents-element-roles.html:
>* LayoutTests/platform/mac-wk2/accessibility/aria-visible-element-roles-expected.txt:
>* Source/WebCore/accessibility/AXObjectCache.cpp:
>(WebCore::shouldCreateAccessibilityList):
>(WebCore::AXObjectCache::createObjectFromRenderer):
>(WebCore::createFromNode):
>(WebCore::AXObjectCache::getOrCreate):
>* Source/WebCore/accessibility/AccessibilityList.cpp:
>(WebCore::AccessibilityList::AccessibilityList):
>(WebCore::AccessibilityList::create):
>* Source/WebCore/accessibility/AccessibilityList.h:
>* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
>(WebCore::AccessibilityNodeObject::checkboxOrRadioRect const):
>(WebCore::AccessibilityNodeObject::elementRect const):
>(WebCore::AccessibilityNodeObject::addChildren):
>(WebCore::AccessibilityNodeObject::canHaveChildren const):
>(WebCore::AccessibilityNodeObject::computeAccessibilityIsIgnored const):
>* Source/WebCore/accessibility/AccessibilityNodeObject.h:
>* Source/WebCore/accessibility/AccessibilityObject.cpp:
>(WebCore::AccessibilityObject::convertFrameToSpace const):
>* Source/WebCore/accessibility/AccessibilityRenderObject.cpp:
>(WebCore::AccessibilityRenderObject::AccessibilityRenderObject):
>(WebCore::AccessibilityRenderObject::firstChild const):
>(WebCore::AccessibilityRenderObject::lastChild const):
>(WebCore::AccessibilityRenderObject::previousSibling const):
>(WebCore::AccessibilityRenderObject::nextSibling const):
>(WebCore::AccessibilityRenderObject::parentObject const):
>(WebCore::AccessibilityRenderObject::helpText const):
>(WebCore::AccessibilityRenderObject::textUnderElement const):
>(WebCore::AccessibilityRenderObject::node const):
>(WebCore::AccessibilityRenderObject::stringValue const):
>(WebCore::AccessibilityRenderObject::boundingBoxRect const):
>(WebCore::AccessibilityRenderObject::defaultObjectInclusion const):
>(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
>(WebCore::AccessibilityRenderObject::document const):
>(WebCore::AccessibilityRenderObject::documentFrameView const):
>(WebCore::AccessibilityRenderObject::addChildren):
>(WebCore::AccessibilityRenderObject::checkboxOrRadioRect const): Deleted.
>(WebCore::AccessibilityRenderObject::elementRect const): Deleted.
>(WebCore::AccessibilityRenderObject::canHaveChildren const): Deleted.
>* Source/WebCore/accessibility/AccessibilityRenderObject.h:
>---
> .../WebCore/accessibility/AXObjectCache.cpp   | 19 +++--
> .../accessibility/AccessibilityList.cpp       | 10 +++
> .../WebCore/accessibility/AccessibilityList.h |  2 +
> .../accessibility/AccessibilityNodeObject.cpp | 36 ++++++--
> .../accessibility/AccessibilityNodeObject.h   |  3 +-
> .../accessibility/AccessibilityObject.cpp     |  2 +-
> .../AccessibilityRenderObject.cpp             | 83 ++++++++-----------
> .../accessibility/AccessibilityRenderObject.h | 10 +--
> .../aria-visible-element-roles.html           |  4 +-
> ...isplay-contents-element-roles-expected.txt |  8 ++
> .../display-contents-element-roles.html       |  7 +-
> .../aria-visible-element-roles-expected.txt   |  9 +-
> ...isplay-contents-element-roles-expected.txt |  4 +
> ...isplay-contents-element-roles-expected.txt |  3 +
> .../aria-visible-element-roles-expected.txt   | 13 +--
> .../aria-visible-element-roles-expected.txt   | 13 +--
> 16 files changed, 134 insertions(+), 92 deletions(-)
>
>diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
>index 2a2f831f1ffd..bf3240bee9ff 100644
>--- a/Source/WebCore/accessibility/AXObjectCache.cpp
>+++ b/Source/WebCore/accessibility/AXObjectCache.cpp
>@@ -591,15 +591,20 @@ static bool isSimpleImage(const RenderObject& renderer)
>     return true;
> }
> 
>+static bool isAccessibilityList(Node* node)
>+{
>+    // If the node is aria role="list" or the aria role is empty and it's a
>+    // ul/ol/dl type (it shouldn't be a list if aria says otherwise).
>+    return (node && ((nodeHasRole(node, "list"_s) || nodeHasRole(node, "directory"_s))
>+        || (nodeHasRole(node, nullAtom()) && (node->hasTagName(ulTag) || node->hasTagName(olTag) || node->hasTagName(dlTag) || node->hasTagName(menuTag)))));
>+}
>+
> Ref<AccessibilityObject> AXObjectCache::createObjectFromRenderer(RenderObject* renderer)
> {
>     // FIXME: How could renderer->node() ever not be an Element?
>     Node* node = renderer->node();
> 
>-    // If the node is aria role="list" or the aria role is empty and its a
>-    // menu/ul/ol/dl type (it shouldn't be a list if aria says otherwise).
>-    if (node && ((nodeHasRole(node, "list"_s) || nodeHasRole(node, "directory"_s))
>-        || (nodeHasRole(node, nullAtom()) && (node->hasTagName(menuTag) || node->hasTagName(ulTag) || node->hasTagName(olTag) || node->hasTagName(dlTag)))))
>+    if (isAccessibilityList(node))
>         return AccessibilityList::create(renderer);
> 
>     // aria tables
>@@ -676,6 +681,8 @@ Ref<AccessibilityObject> AXObjectCache::createObjectFromRenderer(RenderObject* r
> 
> static Ref<AccessibilityObject> createFromNode(Node& node)
> {
>+    if (isAccessibilityList(&node))
>+        return AccessibilityList::create(node);
>     return AccessibilityNodeObject::create(node);
> }
> 
>@@ -736,7 +743,7 @@ AccessibilityObject* AXObjectCache::getOrCreate(Node* node)
> 
>     if (!node->parentElement())
>         return nullptr;
>-    
>+
>     bool isOptionElement = is<HTMLOptionElement>(*node);
>     if (isOptionElement || is<HTMLOptGroupElement>(*node)) {
>         auto select = isOptionElement
>@@ -782,7 +789,7 @@ AccessibilityObject* AXObjectCache::getOrCreate(Node* node)
>     // it will disappear when this function is finished, leading to a use-after-free.
>     if (newObject->isDetached())
>         return nullptr;
>-    
>+
>     return newObject.get();
> }
> 
>diff --git a/Source/WebCore/accessibility/AccessibilityList.cpp b/Source/WebCore/accessibility/AccessibilityList.cpp
>index 2885ee6838ea..78c3270e784e 100644
>--- a/Source/WebCore/accessibility/AccessibilityList.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityList.cpp
>@@ -46,6 +46,11 @@ AccessibilityList::AccessibilityList(RenderObject* renderer)
> {
> }
> 
>+AccessibilityList::AccessibilityList(Node& node)
>+    : AccessibilityRenderObject(node)
>+{
>+}
>+
> AccessibilityList::~AccessibilityList() = default;
> 
> Ref<AccessibilityList> AccessibilityList::create(RenderObject* renderer)
>@@ -53,6 +58,11 @@ Ref<AccessibilityList> AccessibilityList::create(RenderObject* renderer)
>     return adoptRef(*new AccessibilityList(renderer));
> }
> 
>+Ref<AccessibilityList> AccessibilityList::create(Node& node)
>+{
>+    return adoptRef(*new AccessibilityList(node));
>+}
>+
> bool AccessibilityList::computeAccessibilityIsIgnored() const
> {
>     return accessibilityIsIgnoredByDefault();
>diff --git a/Source/WebCore/accessibility/AccessibilityList.h b/Source/WebCore/accessibility/AccessibilityList.h
>index 3a061e8dd598..0e4080bfa4a5 100644
>--- a/Source/WebCore/accessibility/AccessibilityList.h
>+++ b/Source/WebCore/accessibility/AccessibilityList.h
>@@ -35,11 +35,13 @@ namespace WebCore {
> class AccessibilityList final : public AccessibilityRenderObject {
> public:
>     static Ref<AccessibilityList> create(RenderObject*);
>+    static Ref<AccessibilityList> create(Node&);
>     virtual ~AccessibilityList();
> 
>     AccessibilityRole roleValue() const override;
> private:
>     explicit AccessibilityList(RenderObject*);
>+    explicit AccessibilityList(Node&);
>     bool isList() const override { return true; }
>     bool isUnorderedList() const override;
>     bool isOrderedList() const override;
>diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
>index 91c673976a54..2e4d8796b829 100644
>--- a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
>@@ -215,8 +215,27 @@ AccessibilityObject* AccessibilityNodeObject::parentObject() const
>     return nullptr;
> }
> 
>+LayoutRect AccessibilityNodeObject::checkboxOrRadioRect() const
>+{
>+    auto* label = labelForElement(dynamicDowncast<Element>(node()));
>+    if (!label || !label->renderer())
>+        return boundingBoxRect();
>+
>+    auto* cache = axObjectCache();
>+    if (!cache)
>+        return boundingBoxRect();
>+
>+    // A checkbox or radio button should encompass its label.
>+    auto labelRect = cache->getOrCreate(label)->elementRect();
>+    labelRect.unite(boundingBoxRect());
>+    return labelRect;
>+}
>+
> LayoutRect AccessibilityNodeObject::elementRect() const
> {
>+    if (isCheckboxOrRadio())
>+        return checkboxOrRadioRect();
>+
>     return boundingBoxRect();
> }
> 
>@@ -490,13 +509,13 @@ void AccessibilityNodeObject::addChildren()
> {
>     // If the need to add more children in addition to existing children arises, 
>     // childrenChanged should have been called, leaving the object with no children.
>-    ASSERT(!m_childrenInitialized); 
>+    ASSERT(!m_childrenInitialized);
>     
>-    if (!m_node)
>-        return;
>-
>     m_childrenInitialized = true;
> 
>+    if (!canHaveChildren())
>+        return;
>+
>     // The only time we add children from the DOM tree to a node with a renderer is when it's a canvas.
>     if (renderer() && !node()->hasTagName(canvasTag))
>         return;
>@@ -507,7 +526,7 @@ void AccessibilityNodeObject::addChildren()
> 
>     for (auto* child = node()->firstChild(); child; child = child->nextSibling())
>         addChild(objectCache->getOrCreate(child));
>-    
>+
>     updateOwnedChildren();
>     m_subtreeDirty = false;
> }
>@@ -516,8 +535,9 @@ bool AccessibilityNodeObject::canHaveChildren() const
> {
>     // If this is an AccessibilityRenderObject, then it's okay if this object
>     // doesn't have a node - there are some renderers that don't have associated
>-    // nodes, like scroll areas and css-generated text.
>-    if (!node() && !isAccessibilityRenderObject())
>+    // nodes, like scroll areas and css-generated text. However, if there also
>+    // no renderer, we should bail.
>+    if (!renderer() && !node())
>         return false;
> 
>     // When <noscript> is not being used (its renderer() == 0), ignore its children.
>@@ -572,6 +592,8 @@ bool AccessibilityNodeObject::computeAccessibilityIsIgnored() const
>     // it's been initialized.
>     ASSERT(m_initialized);
> #endif
>+    if (!node())
>+        return true;
> 
>     // Handle non-rendered text that is exposed through aria-hidden=false.
>     if (node() && node()->isTextNode() && !renderer()) {
>diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h
>index 7e33383bedc1..56d53bed7532 100644
>--- a/Source/WebCore/accessibility/AccessibilityNodeObject.h
>+++ b/Source/WebCore/accessibility/AccessibilityNodeObject.h
>@@ -161,6 +161,7 @@ protected:
>     virtual AccessibilityRole determineAriaRoleAttribute() const;
>     AccessibilityRole remapAriaRoleDueToParent(AccessibilityRole) const;
> 
>+    bool computeAccessibilityIsIgnored() const override;
>     void addChildren() override;
>     void clearChildren() override;
>     void updateChildrenIfNecessary() override;
>@@ -210,13 +211,13 @@ private:
>     void visibleText(Vector<AccessibilityText>&) const;
>     String alternativeTextForWebArea() const;
>     void ariaLabeledByText(Vector<AccessibilityText>&) const;
>-    bool computeAccessibilityIsIgnored() const override;
>     bool usesAltTagForTextComputation() const;
>     bool roleIgnoresTitle() const;
>     bool postKeyboardKeysForValueChange(StepAction);
>     void setNodeValue(StepAction, float);
>     bool performDismissAction() final;
>     bool hasTextAlternative() const;
>+    LayoutRect checkboxOrRadioRect() const;
> 
>     void setNeedsToUpdateChildren() override { m_childrenDirty = true; }
>     bool needsToUpdateChildren() const override { return m_childrenDirty; }
>diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
>index 2f67c0c2372f..b30ea1eaa38e 100644
>--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp
>@@ -448,7 +448,7 @@ AccessibilityObject* AccessibilityObject::previousSiblingUnignored(int limit) co
> FloatRect AccessibilityObject::convertFrameToSpace(const FloatRect& frameRect, AccessibilityConversionSpace conversionSpace) const
> {
>     ASSERT(isMainThread());
>-    
>+
>     // Find the appropriate scroll view to use to convert the contents to the window.
>     const auto parentAccessibilityScrollView = ancestorAccessibilityScrollView(false /* includeSelf */);
>     auto* parentScrollView = parentAccessibilityScrollView ? parentAccessibilityScrollView->scrollView() : nullptr;
>diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>index f572b711650a..c7d5f85d88b3 100644
>--- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>@@ -132,6 +132,13 @@ AccessibilityRenderObject::AccessibilityRenderObject(RenderObject* renderer)
> #endif
> }
> 
>+AccessibilityRenderObject::AccessibilityRenderObject(Node& node)
>+    : AccessibilityNodeObject(&node)
>+{
>+    // We should only ever create an instance of this class with a node if that node has no renderer (i.e. because of display:contents).
>+    ASSERT(!node.renderer());
>+}
>+
> AccessibilityRenderObject::~AccessibilityRenderObject()
> {
>     ASSERT(isDetached());
>@@ -218,7 +225,7 @@ static inline RenderObject* lastChildConsideringContinuation(RenderObject& rende
> AccessibilityObject* AccessibilityRenderObject::firstChild() const
> {
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::firstChild();
>     
>     RenderObject* firstChild = firstChildConsideringContinuation(*m_renderer);
> 
>@@ -236,7 +243,7 @@ AccessibilityObject* AccessibilityRenderObject::firstChild() const
> AccessibilityObject* AccessibilityRenderObject::lastChild() const
> {
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::lastChild();
> 
>     RenderObject* lastChild = lastChildConsideringContinuation(*m_renderer);
> 
>@@ -313,7 +320,7 @@ static inline bool firstChildIsInlineContinuation(RenderElement& renderer)
> AccessibilityObject* AccessibilityRenderObject::previousSibling() const
> {
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::previousSibling();
> 
>     RenderObject* previousSibling = nullptr;
> 
>@@ -360,7 +367,10 @@ static inline bool lastChildHasContinuation(RenderElement& renderer)
> 
> AccessibilityObject* AccessibilityRenderObject::nextSibling() const
> {
>-    if (!m_renderer || is<RenderView>(*m_renderer))
>+    if (!m_renderer)
>+        return AccessibilityNodeObject::nextSibling();
>+
>+    if (is<RenderView>(m_renderer))
>         return nullptr;
> 
>     RenderObject* nextSibling = nullptr;
>@@ -483,7 +493,7 @@ RenderObject* AccessibilityRenderObject::renderParentObject() const
>         
>     return parent;
> }
>-    
>+
> AccessibilityObject* AccessibilityRenderObject::parentObjectIfExists() const
> {
>     AXObjectCache* cache = axObjectCache();
>@@ -512,7 +522,7 @@ AccessibilityObject* AccessibilityRenderObject::parentObject() const
>         return displayContentsParent;
> 
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::parentObject();
> 
>     WeakPtr cache = axObjectCache();
>     if (!cache)
>@@ -614,7 +624,7 @@ Element* AccessibilityRenderObject::anchorElement() const
> String AccessibilityRenderObject::helpText() const
> {
>     if (!m_renderer)
>-        return { };
>+        return AccessibilityNodeObject::helpText();
> 
>     const auto& ariaHelp = getAttribute(aria_helpAttr);
>     if (!ariaHelp.isEmpty())
>@@ -651,7 +661,7 @@ String AccessibilityRenderObject::helpText() const
> String AccessibilityRenderObject::textUnderElement(AccessibilityTextUnderElementMode mode) const
> {
>     if (!m_renderer)
>-        return String();
>+        return AccessibilityNodeObject::textUnderElement(mode);
> 
>     if (is<RenderFileUploadControl>(*m_renderer))
>         return downcast<RenderFileUploadControl>(*m_renderer).buttonValue();
>@@ -744,17 +754,15 @@ bool AccessibilityRenderObject::shouldGetTextFromNode(AccessibilityTextUnderElem
> 
> Node* AccessibilityRenderObject::node() const
> {
>-    if (!m_renderer)
>-        return nullptr;
>-    if (m_renderer->isRenderView())
>-        return &m_renderer->document();
>-    return m_renderer->node();
>-}    
>+    if (m_renderer)
>+        return LIKELY(!m_renderer->isRenderView()) ? m_renderer->node() : &m_renderer->document();
>+    return AccessibilityNodeObject::node();
>+}
> 
> String AccessibilityRenderObject::stringValue() const
> {
>     if (!m_renderer)
>-        return { };
>+        return AccessibilityNodeObject::stringValue();
> 
>     if (isStaticText() || isTextControl() || isSecureField())
>         return text();
>@@ -826,7 +834,7 @@ LayoutRect AccessibilityRenderObject::boundingBoxRect() const
>     RenderObject* obj = renderer();
>     
>     if (!obj)
>-        return LayoutRect();
>+        return AccessibilityNodeObject::boundingBoxRect();
>     
>     if (obj->node()) // If we are a continuation, we want to make sure to use the primary renderer.
>         obj = obj->node()->renderer();
>@@ -860,29 +868,6 @@ LayoutRect AccessibilityRenderObject::boundingBoxRect() const
>     return result;
> }
>     
>-LayoutRect AccessibilityRenderObject::checkboxOrRadioRect() const
>-{
>-    if (!m_renderer)
>-        return LayoutRect();
>-    
>-    HTMLLabelElement* label = labelForElement(downcast<Element>(m_renderer->node()));
>-    if (!label || !label->renderer())
>-        return boundingBoxRect();
>-    
>-    LayoutRect labelRect = axObjectCache()->getOrCreate(label)->elementRect();
>-    labelRect.unite(boundingBoxRect());
>-    return labelRect;
>-}
>-
>-LayoutRect AccessibilityRenderObject::elementRect() const
>-{
>-    // a checkbox or radio button should encompass its label
>-    if (isCheckboxOrRadio())
>-        return checkboxOrRadioRect();
>-    
>-    return boundingBoxRect();
>-}
>-
> bool AccessibilityRenderObject::supportsPath() const
> {
>     return is<RenderText>(renderer()) || (renderer() && renderer()->isSVGShapeOrLegacySVGShape());
>@@ -1064,7 +1049,7 @@ static AccessibilityObjectInclusion objectInclusionFromAltText(const String& alt
> 
> AccessibilityObjectInclusion AccessibilityRenderObject::defaultObjectInclusion() const
> {
>-    if (!m_renderer)
>+    if (!m_renderer && !node())
>         return AccessibilityObjectInclusion::IgnoreObject;
>     return AccessibilityNodeObject::defaultObjectInclusion();
> }
>@@ -1087,7 +1072,7 @@ bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
> #endif
> 
>     if (!m_renderer)
>-        return true;
>+        return AccessibilityNodeObject::computeAccessibilityIsIgnored();
> 
>     // Check first if any of the common reasons cause this element to be ignored.
>     // Then process other use cases that need to be applied to all the various roles
>@@ -1528,7 +1513,7 @@ RenderView* AccessibilityRenderObject::topRenderer() const
> Document* AccessibilityRenderObject::document() const
> {
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::document();
>     return &m_renderer->document();
> }
> 
>@@ -1602,9 +1587,8 @@ AXCoreObject::AccessibilityChildrenVector AccessibilityRenderObject::documentLin
> LocalFrameView* AccessibilityRenderObject::documentFrameView() const 
> { 
>     if (!m_renderer)
>-        return nullptr;
>+        return AccessibilityNodeObject::documentFrameView();
> 
>-    // this is the RenderObject's Document's Frame's LocalFrameView
>     return &m_renderer->view().frameView();
> }
> 
>@@ -2603,7 +2587,11 @@ void AccessibilityRenderObject::updateRoleAfterChildrenCreation()
>     
> void AccessibilityRenderObject::addChildren()
> {
>-    // If the need to add more children in addition to existing children arises, 
>+    if (UNLIKELY(!renderer())) {
>+        AccessibilityNodeObject::addChildren();
>+        return;
>+    }
>+    // If the need to add more children in addition to existing children arises,
>     // childrenChanged should have been called, leaving the object with no children.
>     ASSERT(!m_childrenInitialized); 
> 
>@@ -2645,11 +2633,6 @@ void AccessibilityRenderObject::addChildren()
>     updateRoleAfterChildrenCreation();
> }
> 
>-bool AccessibilityRenderObject::canHaveChildren() const
>-{
>-    return m_renderer && AccessibilityNodeObject::canHaveChildren();
>-}
>-
> void AccessibilityRenderObject::setAccessibleName(const AtomString& name)
> {
>     // Setting the accessible name can store the value in the DOM
>diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.h b/Source/WebCore/accessibility/AccessibilityRenderObject.h
>index 46e965700f0d..4ca7139d39b0 100644
>--- a/Source/WebCore/accessibility/AccessibilityRenderObject.h
>+++ b/Source/WebCore/accessibility/AccessibilityRenderObject.h
>@@ -88,17 +88,14 @@ public:
>     Element* anchorElement() const override;
>     
>     LayoutRect boundingBoxRect() const override;
>-    LayoutRect elementRect() const override;
> 
>     RenderObject* renderer() const override { return m_renderer.get(); }
>     RenderBoxModelObject* renderBoxModelObject() const;
>     Node* node() const override;
> 
>     Document* document() const override;
>-
>     RenderView* topRenderer() const;
>-    RenderTextControl* textControl() const;
>-    
>+
>     URL url() const override;
>     PlainTextRange selectedTextRange() const override;
>     int insertionPointLineNumber() const override;
>@@ -117,7 +114,6 @@ public:
>     bool setValue(const String&) override;
> 
>     void addChildren() override;
>-    bool canHaveChildren() const override;
> 
>     VisiblePositionRange visiblePositionRange() const override;
>     VisiblePositionRange visiblePositionRangeForLine(unsigned) const override;
>@@ -145,11 +141,12 @@ public:
> 
> protected:
>     explicit AccessibilityRenderObject(RenderObject*);
>+    explicit AccessibilityRenderObject(Node&);
>     void detachRemoteParts(AccessibilityDetachmentType) override;
>     ScrollableArea* getScrollableAreaIfScrollable() const override;
>     void scrollTo(const IntPoint&) const override;
>     
>-    bool isDetached() const override { return !m_renderer; }
>+    bool isDetached() const final { return !m_renderer && AccessibilityNodeObject::isDetached(); }
> 
>     bool shouldIgnoreAttributeRole() const override;
>     AccessibilityRole determineAccessibilityRole() override;
>@@ -169,7 +166,6 @@ private:
>     bool nodeIsTextControl(const Node*) const;
>     Path elementPath() const override;
>     
>-    LayoutRect checkboxOrRadioRect() const;
>     AXCoreObject* accessibilityImageMapHitTest(HTMLAreaElement*, const IntPoint&) const;
>     AccessibilityObject* accessibilityParentForImageMap(HTMLMapElement*) const;
>     AXCoreObject* elementAccessibilityHitTest(const IntPoint&) const override;
>diff --git a/LayoutTests/accessibility/aria-visible-element-roles.html b/LayoutTests/accessibility/aria-visible-element-roles.html
>index 05de7c351aa6..a905a04ed709 100644
>--- a/LayoutTests/accessibility/aria-visible-element-roles.html
>+++ b/LayoutTests/accessibility/aria-visible-element-roles.html
>@@ -111,7 +111,9 @@
> 
>     <output hidden aria-hidden="false" id="output">Output</output>
> 
>-    <menu hidden aria-hidden="false" type="toolbar" id="menu-toolbar"></menu>
>+    <menu hidden aria-hidden="false" type="toolbar" id="menu-toolbar">
>+        <li hidden aria-hidden="false">Hello world</li>
>+    </menu>
> 
>     <hr hidden aria-hidden="false" id="hr"></hr>
> 
>diff --git a/LayoutTests/accessibility/display-contents-element-roles-expected.txt b/LayoutTests/accessibility/display-contents-element-roles-expected.txt
>index f8bdce49a055..6fa5385d9a6f 100644
>--- a/LayoutTests/accessibility/display-contents-element-roles-expected.txt
>+++ b/LayoutTests/accessibility/display-contents-element-roles-expected.txt
>@@ -51,6 +51,7 @@ This test ensures elements with CSS display: contents have the correct role.
> 
> <dl class="testcase" id="dl"></dl>
>     AXRole: AXList
>+    AXSubrole: AXDescriptionList
> 
> <dt class="testcase" id="dt"></dt>
>     AXRole: AXGroup
>@@ -102,6 +103,11 @@ This test ensures elements with CSS display: contents have the correct role.
> <menu class="testcase" type="toolbar" id="menu-toolbar"></menu>
>     AXRole: AXList
>     computedRoleString: list
>+    AXSubrole: AXContentList
>+
>+<li class="testcase" id="ul-li-element"></li>
>+    AXRole: AXGroup
>+    computedRoleString: listitem
> 
> <nav class="testcase" id="nav"></nav>
>     AXRole: AXGroup
>@@ -111,6 +117,7 @@ This test ensures elements with CSS display: contents have the correct role.
> <ol class="testcase" id="ol"></ol>
>     AXRole: AXList
>     computedRoleString: list
>+    AXSubrole: AXContentList
> 
> <li class="testcase" id="ol-li-element"></li>
>     AXRole: AXGroup
>@@ -155,6 +162,7 @@ This test ensures elements with CSS display: contents have the correct role.
> <ul class="testcase" id="ul"></ul>
>     AXRole: AXList
>     computedRoleString: list
>+    AXSubrole: AXContentList
> 
> <li class="testcase" id="ul-li-element"></li>
>     AXRole: AXGroup
>diff --git a/LayoutTests/accessibility/display-contents-element-roles.html b/LayoutTests/accessibility/display-contents-element-roles.html
>index f0cc3f6549b4..b9840ee7daeb 100644
>--- a/LayoutTests/accessibility/display-contents-element-roles.html
>+++ b/LayoutTests/accessibility/display-contents-element-roles.html
>@@ -63,12 +63,16 @@
> 
>     <mark class="testcase" id="mark">Marked text</mark>
> 
>-    <menu class="testcase" type="toolbar" id="menu-toolbar"></menu>
>+    <menu class="testcase" type="toolbar" id="menu-toolbar">
>+        <li class="testcase" id="ul-li-element">Hello world</li>
>+        <li>list item with renderer</li>
>+    </menu>
> 
>     <nav class="testcase" id="nav">Nav</nav>
> 
>     <ol class="testcase" id="ol">
>         <li class="testcase" id="ol-li-element">Hello world</li>
>+        <li>list item with renderer</li>
>     </ol>
> 
>     <output class="testcase" id="output">Output</output>
>@@ -89,6 +93,7 @@
> 
>     <ul class="testcase" id="ul">
>         <li class="testcase" id="ul-li-element">Hello world</li>
>+        <li>list item with renderer</li>
>     </ul>
> </div>
> 
>diff --git a/LayoutTests/platform/glib/accessibility/aria-visible-element-roles-expected.txt b/LayoutTests/platform/glib/accessibility/aria-visible-element-roles-expected.txt
>index f12511f61957..f741414372c5 100644
>--- a/LayoutTests/platform/glib/accessibility/aria-visible-element-roles-expected.txt
>+++ b/LayoutTests/platform/glib/accessibility/aria-visible-element-roles-expected.txt
>@@ -105,12 +105,10 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
>     AXRole: AXDescriptionList
> 
> <ol hidden="" aria-hidden="false" id="ol"></ol>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
> 
> <ul hidden="" aria-hidden="false" id="ul"></ul>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
> 
> <figure hidden="" aria-hidden="false" id="figure"></figure>
>     AXRole: AXGroup
>@@ -180,8 +178,7 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
>     computedRoleString: status
> 
> <menu hidden="" aria-hidden="false" type="toolbar" id="menu-toolbar"></menu>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
> 
> <hr hidden="" aria-hidden="false" id="hr">
>     AXRole: AXSeparator
>diff --git a/LayoutTests/platform/glib/accessibility/display-contents-element-roles-expected.txt b/LayoutTests/platform/glib/accessibility/display-contents-element-roles-expected.txt
>index 98f2265607e6..dd87bbdaf41d 100644
>--- a/LayoutTests/platform/glib/accessibility/display-contents-element-roles-expected.txt
>+++ b/LayoutTests/platform/glib/accessibility/display-contents-element-roles-expected.txt
>@@ -89,6 +89,10 @@ This test ensures elements with CSS display: contents have the correct role.
>     AXRole: AXList
>     computedRoleString: list
> 
>+<li class="testcase" id="ul-li-element"></li>
>+    AXRole: AXListItem
>+    computedRoleString: listitem
>+
> <nav class="testcase" id="nav"></nav>
>     AXRole: AXLandmarkNavigation
>     computedRoleString: navigation
>diff --git a/LayoutTests/platform/ios/accessibility/display-contents-element-roles-expected.txt b/LayoutTests/platform/ios/accessibility/display-contents-element-roles-expected.txt
>index 12f1c7dd0256..d79b003e88d9 100644
>--- a/LayoutTests/platform/ios/accessibility/display-contents-element-roles-expected.txt
>+++ b/LayoutTests/platform/ios/accessibility/display-contents-element-roles-expected.txt
>@@ -72,6 +72,9 @@ This test ensures elements with CSS display: contents have the correct role.
> <menu class="testcase" type="toolbar" id="menu-toolbar"></menu>
>     List
> 
>+<li class="testcase" id="ul-li-element"></li>
>+    ListItem
>+
> <nav class="testcase" id="nav"></nav>
>     LandmarkNavigation
> 
>diff --git a/LayoutTests/platform/mac-wk1/accessibility/aria-visible-element-roles-expected.txt b/LayoutTests/platform/mac-wk1/accessibility/aria-visible-element-roles-expected.txt
>index c93c0b3d4dbf..48620af41462 100644
>--- a/LayoutTests/platform/mac-wk1/accessibility/aria-visible-element-roles-expected.txt
>+++ b/LayoutTests/platform/mac-wk1/accessibility/aria-visible-element-roles-expected.txt
>@@ -114,14 +114,15 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
> 
> <dl hidden="" aria-hidden="false" id="dl"></dl>
>     AXRole: AXList
>+    AXSubrole: AXDescriptionList
> 
> <ol hidden="" aria-hidden="false" id="ol"></ol>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <ul hidden="" aria-hidden="false" id="ul"></ul>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <figure hidden="" aria-hidden="false" id="figure"></figure>
>     AXRole: AXGroup
>@@ -203,8 +204,8 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
>     AXSubrole: AXApplicationStatus
> 
> <menu hidden="" aria-hidden="false" type="toolbar" id="menu-toolbar"></menu>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <hr hidden="" aria-hidden="false" id="hr">
>     AXRole: AXSplitter
>diff --git a/LayoutTests/platform/mac-wk2/accessibility/aria-visible-element-roles-expected.txt b/LayoutTests/platform/mac-wk2/accessibility/aria-visible-element-roles-expected.txt
>index f282344a20a0..c1b9b1047788 100644
>--- a/LayoutTests/platform/mac-wk2/accessibility/aria-visible-element-roles-expected.txt
>+++ b/LayoutTests/platform/mac-wk2/accessibility/aria-visible-element-roles-expected.txt
>@@ -113,14 +113,15 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
> 
> <dl hidden="" aria-hidden="false" id="dl"></dl>
>     AXRole: AXList
>+    AXSubrole: AXDescriptionList
> 
> <ol hidden="" aria-hidden="false" id="ol"></ol>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <ul hidden="" aria-hidden="false" id="ul"></ul>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <figure hidden="" aria-hidden="false" id="figure"></figure>
>     AXRole: AXGroup
>@@ -202,8 +203,8 @@ This test ensures ARIA visible elements (e.g. those with both `hidden` and `aria
>     AXSubrole: AXApplicationStatus
> 
> <menu hidden="" aria-hidden="false" type="toolbar" id="menu-toolbar"></menu>
>-    AXRole: AXList
>-    computedRoleString: list
>+    AXRole: AXGroup
>+    AXSubrole: AXContentList
> 
> <hr hidden="" aria-hidden="false" id="hr">
>     AXRole: AXSplitter

Actions: View | Formatted Diff | Diff

Attachments on bug 256950: 466397 | 466438 | 466439 | 466440 | 466484 | 466511 | 466526