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

Ali Juma
Reported 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.
Attachments
Patch (21.28 KB, patch)
2018-08-21 13:39 PDT, Ali Juma 		no flags
DetailsFormatted DiffDiff
Patch for landing (21.26 KB, patch)
2018-08-21 15:29 PDT, Ali Juma 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ali Juma
Comment 1 2018-08-21 13:39:10 PDT
Created attachment 347684 [details] Patch
Ali Juma
Comment 2 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.
Simon Fraser (smfr)
Comment 3 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.
Ali Juma
Comment 4 2018-08-21 15:29:15 PDT
Created attachment 347712 [details] Patch for landing
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2018-08-27 04:13:59 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2018-08-27 04:14:19 PDT
<rdar://problem/43749632>
Note You need to log in before you can comment on or make changes to this bug.