Implement closed shadow adjustment for Element.offsetParent

third_party/WebKit/Source/core/layout/LayoutObject.cpp

Index: third_party/WebKit/Source/core/layout/LayoutObject.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.cpp b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
index 7b7996ca542c27b32613429569ccd690bd5d2d9f..9fb1ca483578a07ddf63457994a5911a5920be4a 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
@@ -3244,7 +3244,7 @@ void LayoutObject::imageChanged(ImageResource* image, const IntRect* rect)
     imageChanged(static_cast<WrappedImagePtr>(image), rect);
 }
 
-Element* LayoutObject::offsetParent() const
+Element* LayoutObject::offsetParentInternal() const
 {
     if (isDocumentElement() || isBody())
         return nullptr;
@@ -3279,6 +3279,16 @@ Element* LayoutObject::offsetParent() const
     return node && node->isElementNode() ? toElement(node) : nullptr;
 }
 
+Element* LayoutObject::offsetParent() const

[hayato, 2016/06/09 08:22:03]

A binding layer is the only client for this method?
Unless that, do not change the behavior of LayoutObject::offsetParent().
That would break something if there are other callers than bindings.

Rather, we should have Element::offsetParentForBinding(), and remove *unclosed
check* from Element::offsetParent and LayoutObject::offsetParent().

[kochi, 2016/06/09 13:09:28]

Good catch!

Currently Element::offsetParent() is only used for HTMLElement.offsetParent
binding
so it should be fine, but LayoutObject::offsetParent() is used for other
purposes,
thus this may break something.

So I separated LayoutObject::offsetParent() into offsetParent() and
offsetParentForBinding(), because LayoutBoxModelObject::offsetTop() et al.
depend on the returned element of offsetParentForBinding() - otherwise
Element.offsetTop et al. would return wrong offset.

The logic in the original offsetParent() looks quite tied with the CSSOM spec,
so I'm not sure how other users (e.g. spatial navigation and hit-test region
visualizer) depend on the Shadow DOM unclosed node behavior.
But I guess spatial navigation should work regardless of Shadow DOM, so keeping
the original offsetParent() should make sense here.

I'll ask for another look from layout team reviewer.

+{
+    Element* element = offsetParentInternal();
+
+    while (element && !element->isUnclosedNodeOf(*node()))
+        element = element->layoutObject()->offsetParent();
+
+    return element;
+}
+
 PositionWithAffinity LayoutObject::createPositionWithAffinity(int offset, TextAffinity affinity)
 {
     // If this is a non-anonymous layoutObject in an editable area, then it's simple.