[webkit-reviews] review granted: [Bug 157437] offsetLeft and offsetParent should adjust across shadow boundaries : [Attachment 357503] Fixed GTK+ build failures

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 157437: offsetLeft and offsetParent should adjust across shadow boundaries
https://bugs.webkit.org/show_bug.cgi?id=157437

Attachment 357503: Fixed GTK+ build failures

https://bugs.webkit.org/attachment.cgi?id=357503&action=review




--- Comment #11 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 357503
  --> https://bugs.webkit.org/attachment.cgi?id=357503
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().