Because display:contents AccessibilityNodeObjects can have rendered content (unlike `hidden`, `aria-hidden="false"` node objects), we can compute AccessibilityNodeObject::elementRect by adding up the rectangles of the object's children.
<rdar://problem/91177670>
Created attachment 456393 [details] Patch
Comment on attachment 456393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456393&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:227 > + if (is<AccessibilityNodeObject>(ancestor)) since AXRenderObject inherits from AXNodeObject isn't this true for render objects? is this better as if (!is<AccessibilityRenderObject>) { continue
Created attachment 456397 [details] Patch
(In reply to chris fleizach from comment #3) > Comment on attachment 456393 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=456393&action=review > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:227 > > + if (is<AccessibilityNodeObject>(ancestor)) > > since AXRenderObject inherits from AXNodeObject isn't this true for render > objects? > is this better as > > if (!is<AccessibilityRenderObject>) { continue Yes, thanks!
Comment on attachment 456397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456397&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:212 > + auto childrenCopy = m_children; are we actually doing any copying here? or can we just iteration m_children?
(In reply to chris fleizach from comment #6) > Comment on attachment 456397 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=456397&action=review > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:212 > > + auto childrenCopy = m_children; > > are we actually doing any copying here? or can we just iteration m_children? Yes, I think the copy is not necessary since m_children is a Vector<RefPtr<AXCoreObject>> so the objects are not going away since they are held by the RefPtr.
(In reply to Andres Gonzalez from comment #7) > (In reply to chris fleizach from comment #6) > > Comment on attachment 456397 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=456397&action=review > > > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:212 > > > + auto childrenCopy = m_children; > > > > are we actually doing any copying here? or can we just iteration m_children? > > Yes, I think the copy is not necessary since m_children is a > Vector<RefPtr<AXCoreObject>> so the objects are not going away since they > are held by the RefPtr. So I think you can do just: + for (const auto& child : children(false))
Created attachment 456508 [details] Patch
> So I think you can do just: > > + for (const auto& child : children(false)) We can't use children() (even children(false)) because AXCoreObject::children is not const, while boundingBoxRect is. So we can loop through m_children, which is essentially the same as children(false).
Created attachment 456525 [details] Patch
(In reply to Tyler Wilcock from comment #10) > > So I think you can do just: > > > > + for (const auto& child : children(false)) > We can't use children() (even children(false)) because > AXCoreObject::children is not const, while boundingBoxRect is. So we can > loop through m_children, which is essentially the same as children(false). The problem with that is that a subclass of AXNodeObject now or in the future may override children() and do something that is not equivalent to just m_children. A workaround for the const issue is to use const_cast as ugly as it is.
Created attachment 456566 [details] Patch
Committed r292314 (249207@main): <https://commits.webkit.org/249207@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 456566 [details].