Bug 188809

Summary: [IntersectionObserver] Implement intersection logic for the explicit root case
Product: WebKit Reporter: Ali Juma <ajuma>
Component: Layout and RenderingAssignee: Ali Juma <ajuma>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 159475    
Attachments:
Description Flags
Patch
none
Patch for landing none

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>