Bug 165746 – Allow cross-document intersection observing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Simon Fraser (smfr)]

My first version of Intersection Observer will disallow cross-document observing, for simplicity. This bug tracks supporting cross-document observation.

[Jeffrey Gilbert]

It's a tragedy this wont land in iOS 11 or Safari 11, those being non-evergreen browsers. My first request for this functionality was in 2014 and Safari is the only major browser to not implement a mechanism for cross document in view measurement. There are mechanisms for measurement all the way back to IE9 and tons of destructive/invasive hacks like frame busting and geometric guessing that are being used today, including in some cases measurement using plugins. 

It seems this is somehow a lower priority item than other enhancement requests, but Safari is most mobile web traffic and still relevant in desktop browser markets as well. So long as those browsers don't report in view dims, fraudsters will target iOS applications and safari web browsers, and safe measurement techniques will not be available, making energy/computationally expensive and potentially unsafe operations for measurement the only way to combat bad actors on the web.

[Jeffrey Gilbert]

For more information about "Viewability" measurement guidelines, see the following pages provided by standards leaders in advertising, IAB and MRC found here:

https://www.iab.com/guidelines/iab-measurement-guidelines/

Desktop: http://mediaratingcouncil.org/Desktop-Display-Impression-Measurement-Guidelines-US%20(MMTF%20Public%20Comment%20Final).pdf

Mobile App: http://mediaratingcouncil.org/Mobile%20In-App%20Measurement%20Guidelines%20(MMTF%20Public%20Comment%20Draft%20Final).pdf

Mobile Web: http://mediaratingcouncil.org/Mobile%20Web%20Measurement%20Guidelines%20(MMTF%20Public%20Comment%20Draft%20Final).pdf

Mobile (Guidance): http://mediaratingcouncil.org/062816%20Mobile%20Viewable%20Guidelines%20Final.pdf

WICG developed standard and examples: https://github.com/WICG/IntersectionObserver

And here's where we stand today regarding universal availability on browsers: http://caniuse.com/#feat=intersectionobserver

A deep dive (video) into ad fraud can be found here: https://www.youtube.com/watch?v=tgtH1L57iLc

All this is to say, we very much are ready for this feature out in the wild. It's sorely needed on Safari browsers, and it is a high value feature for the mobile web and mobile app ecosystems and this helps reduce payouts for bad actors.

[Radar WebKit Bug Importer]

<rdar://problem/33381360>

[Jack Wellborn]

Hi Simon, I am stoked to see IntersectionObserver land as an experimental feature in (In reply to Simon Fraser (smfr) from comment #0)
> My first version of Intersection Observer will disallow cross-document
> observing, for simplicity. This bug tracks supporting cross-document
> observation.

Hi Simon, I am stoked to see IntersectionObserver land as an experimental feature in STP. I didn't notice this post until after I made this page https://jackwellborn.com/playground/Webkit/IntersectionObserver/top.html and saw the NaN for the iFrame. Do you have any sense of whether this feature will be released without cross-document support? In either case, is there any ballpark of when cross-document will be supported.

Thanks,

~Jack

[Ali Juma]

Created attachment 353308 [details]
Patch

[EWS Watchlist]

Comment on attachment 353308 [details]
Patch

Attachment 353308 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9770994

New failing tests:
imported/w3c/web-platform-tests/intersection-observer/iframe-no-root.html
imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.html

[EWS Watchlist]

Created attachment 353322 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Ali Juma]

Created attachment 353323 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 353308 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:7615
> +static FloatRect convertFromRootContentsToFrameContents(const FloatRect& rootRect, const FrameView& frameView)
> +{
> +    if (frameView.frame().isMainFrame())
> +        return rootRect;
> +
> +    auto* ownerRenderer = frameView.frame().ownerRenderer();
> +    auto& parentFrameView = ownerRenderer->view().frameView();
> +    FloatRect parentAbsoluteRect = convertFromRootContentsToFrameContents(rootRect, parentFrameView);
> +    LayoutRect rect = LayoutRect(ownerRenderer->absoluteToLocalQuad(parentAbsoluteRect).boundingBox());
> +    rect.moveBy(-ownerRenderer->contentBoxLocation());
> +    return frameView.viewToContents(snappedIntRect(rect));
> +}

This seems to belong in the family of rootViewToContents/contentsToRootView etc. in ScrollView.h. Maybe put this there? We should probably have FloatRect versions of all those.

[Ali Juma]

Created attachment 353529 [details]
Patch

[Ali Juma]

Comment on attachment 353308 [details]
Patch

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

>> Source/WebCore/dom/Document.cpp:7615
>> +}
> 
> This seems to belong in the family of rootViewToContents/contentsToRootView etc. in ScrollView.h. Maybe put this there? We should probably have FloatRect versions of all those.

Removed this, added a FloatRect version of rootViewToContents, and used that instead.

[Simon Fraser (smfr)]

Comment on attachment 353529 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:7691
> +            FloatRect rootViewIntersectionRect = frameView.delegatesScrolling() ? rootAbsoluteIntersectionRect : frameView.contentsToView(rootAbsoluteIntersectionRect);

Is this correct on iOS WK2?

> Source/WebCore/page/FrameView.cpp:2020
> +        if (auto* page = frame().page()) {
> +            if (document->numberOfIntersectionObservers())
>                  page->addDocumentNeedingIntersectionObservationUpdate(*document);
> +            if (!frame().isMainFrame()) {
> +                if (auto* mainDocument = frame().mainFrame().document()) {
> +                    if (mainDocument->numberOfIntersectionObservers())
> +                        page->addDocumentNeedingIntersectionObservationUpdate(*mainDocument);
> +                }
> +            }

Does this handle the case of a intermediate frame (with no observers) changing? Also who triggers observers when an intermediate frame scrolls? Is there a test for that?

[Ali Juma]

Comment on attachment 353529 [details]
Patch

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

>> Source/WebCore/dom/Document.cpp:7691
>> +            FloatRect rootViewIntersectionRect = frameView.delegatesScrolling() ? rootAbsoluteIntersectionRect : frameView.contentsToView(rootAbsoluteIntersectionRect);
> 
> Is this correct on iOS WK2?

Yes, the delegatesScrolling() condition makes this work on iOS WK2.

>> Source/WebCore/page/FrameView.cpp:2020
>> +            }
> 
> Does this handle the case of a intermediate frame (with no observers) changing? Also who triggers observers when an intermediate frame scrolls? Is there a test for that?

Yes, this handles that case. When an intermediate frame scrolls or has its layout change for other reasons, it gets a FrameView::viewportContentsChanged call, and the logic here will ensure the main frame updates its observers (all cross-document observations are tracked by the main frame).

I'll add a test for this case before landing.

[Ali Juma]

Created attachment 353701 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 353701 [details]
Patch for landing

Clearing flags on attachment: 353701

Committed r237737: <https://trac.webkit.org/changeset/237737>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.