Bug 188809 - [IntersectionObserver] Implement intersection logic for the explicit root case
Summary: [IntersectionObserver] Implement intersection logic for the explicit root case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ali Juma
URL:
Keywords: InRadar
Depends on:
Blocks: 159475
  Show dependency treegraph
 
Reported: 2018-08-21 13:31 PDT by Ali Juma
Modified: 2018-08-27 04:14 PDT (History)
5 users (show)

See Also:


Attachments
Patch (21.28 KB, patch)
2018-08-21 13:39 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Patch for landing (21.26 KB, patch)
2018-08-21 15:29 PDT, Ali Juma
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Juma 2018-08-21 13:31:09 PDT
Implement logic to compute the intersection of the target and root, for the case where the observer has a root element.
Comment 1 Ali Juma 2018-08-21 13:39:10 PDT
Created attachment 347684 [details]
Patch
Comment 2 Ali Juma 2018-08-21 13:41:46 PDT
Comment on attachment 347684 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347684&action=review

> LayoutTests/imported/w3c/web-platform-tests/intersection-observer/containing-block-expected.txt:3
> +FAIL In containing block and intersecting. assert_equals: entries[0].boundingClientRect.top expected 18 but got 258

This fails because layout hasn't been updated when the intersection observation is computed. This will be fixed once observations are deferred until after layout.

> LayoutTests/imported/w3c/web-platform-tests/intersection-observer/isIntersecting-change-events-expected.txt:3
> +FAIL Rects in initial notifications should report initial positions. assert_equals: entries[2].target.isIntersecting equals true expected true but got false

This fails because entries[2] is case involving zero-area intersection, which isn't implemented yet.
Comment 3 Simon Fraser (smfr) 2018-08-21 15:20:46 PDT
Comment on attachment 347684 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347684&action=review

> Source/WebCore/dom/Document.cpp:7443
> +static void computeIntersectionObservation(IntersectionObserver& observer, Element* target, FloatRect& absTargetRect, FloatRect& absIntersectionRect, FloatRect& absRootBounds)

I'm not a big fan of the name. Maybe just computeIntersections() or computeIntersectionRects()? Make target an Element& since you don't null-check it.

> Source/WebCore/dom/Document.cpp:7454
> +    RenderBlock* rootRenderer = downcast<RenderBlock>(observer.root()->renderer());

Blank line above this please.

> Source/WebCore/dom/Document.cpp:7485
> +    absTargetRect = targetRenderer->localToAbsoluteQuad(FloatRect(localTargetBounds)).boundingBox();
> +    absRootBounds = rootRenderer->localToAbsoluteQuad(localRootBounds).boundingBox();

It's a shame that these two localToAbsoluteQuad() calls will both pay the cost of traversing the common ancestors of 'target' and 'root'. Not worth optimizing now, though.
Comment 4 Ali Juma 2018-08-21 15:29:15 PDT
Created attachment 347712 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2018-08-27 04:13:57 PDT
Comment on attachment 347712 [details]
Patch for landing

Clearing flags on attachment: 347712

Committed r235358: <https://trac.webkit.org/changeset/235358>
Comment 6 WebKit Commit Bot 2018-08-27 04:13:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2018-08-27 04:14:19 PDT
<rdar://problem/43749632>