Bug 157437

Filed https://github.com/w3c/webcomponents/issues/497 for the spec work.

<rdar://problem/26154021>

Created attachment 357294 [details] WIP

Almost there but I still have a bug that my loop can skip over the offset parent. Basically, we need to find the first offset parent which is a shadow inclusive ancestor so I might just need to check that directly instead of relying on the retargeting algorithm.

FYI, our offsetParent algorithm doesn't match the spec https://github.com/w3c/csswg-drafts/issues/409, bug 161788.

(In reply to Simon Fraser (smfr) from comment #5) > FYI, our offsetParent algorithm doesn't match the spec > https://github.com/w3c/csswg-drafts/issues/409, bug 161788. Oh, interesting. This patch mostly preserves the behavior of offsetParent though. Any quirks it has will be preserved. I'm basically walking offset parents until I find the shadow-including ancestor node.

Created attachment 357478 [details] Fixes the implementation

Created attachment 357503 [details] Fixed GTK+ build failures

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

(In reply to Build Bot from comment #9) > Thanks for the patch. If this patch contains new public API please make sure > it follows the guidelines for new WebKit2 GTK+ API. See > http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Note that this patch does not add or remove a new GTK+ API. It simply updates its implementation to match the JavaScript behavior.

Comment on attachment 357503 [details] Fixed GTK+ build failures View in context: https://bugs.webkit.org/attachment.cgi?id=357503&action=review > Source/WebCore/ChangeLog:9 > + Updates our implementation of offsetLeft, offsetTop, and offsetParent to match the latest discussion in CSS WG. Make it clear this is about offset* in shadow roots; you're not fixing the offsetParent algorithm in general. > Source/WebCore/dom/Element.cpp:914 > + auto parent = makeRefPtr(offsetParent()); > + if (!parent || !parent->isInShadowTree()) > + return offsetLeft(); > + > + ASSERT(&parent->document() == &document()); > + if (&parent->treeScope() == &treeScope()) > + return offsetLeft(); > + > + double offset = offsetLeft(); It would be nice to avoid the 3 separate call sites for offsetLeft(). You could move 'double offset = offsetLeft();' to after the updateLayoutIgnorePendingStylesheets(). > Source/WebCore/dom/Element.cpp:926 > + document().updateLayoutIgnorePendingStylesheets(); Sucks that both offsetLeft and offsetLeftForBindings() call updateLayoutIgnorePendingStylesheets().

(In reply to Simon Fraser (smfr) from comment #11) > Comment on attachment 357503 [details] > Fixed GTK+ build failures > > View in context: > https://bugs.webkit.org/attachment.cgi?id=357503&action=review > > > Source/WebCore/ChangeLog:9 > > + Updates our implementation of offsetLeft, offsetTop, and offsetParent to match the latest discussion in CSS WG. > > Make it clear this is about offset* in shadow roots; you're not fixing the > offsetParent algorithm in general. Phrased as follows: Update the WebKit's treatment of shadow boundaries in offsetLeft, offsetTop, and offsetParent to match the latest discussion in CSS WG. > > Source/WebCore/dom/Element.cpp:914 > > + auto parent = makeRefPtr(offsetParent()); > > + if (!parent || !parent->isInShadowTree()) > > + return offsetLeft(); > > + > > + ASSERT(&parent->document() == &document()); > > + if (&parent->treeScope() == &treeScope()) > > + return offsetLeft(); > > + > > + double offset = offsetLeft(); > > It would be nice to avoid the 3 separate call sites for offsetLeft(). You > could move 'double offset = offsetLeft();' to after the > updateLayoutIgnorePendingStylesheets(). Done that. > > Source/WebCore/dom/Element.cpp:926 > > + document().updateLayoutIgnorePendingStylesheets(); > > Sucks that both offsetLeft and offsetLeftForBindings() call > updateLayoutIgnorePendingStylesheets(). We don't need this now that we always call offsetTop/offsetLeft upfront.

Thanks for the review!

Committed r239313: <https://trac.webkit.org/changeset/239313>