Summary: | Allow cross-document intersection observing | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Ali Juma <ajuma> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ajuma, cdumez, cigitia, commit-queue, dave, dbates, esprehn+autocc, ews-watchlist, jeffreytgilbert, jonlee, jshannon, kangil.han, simon.fraser, w0nka, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 190493 | ||||||||||||||
Bug Blocks: | 159475 | ||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2016-12-11 20:35:57 PST
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. 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. 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 Created attachment 353308 [details]
Patch
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 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
Created attachment 353323 [details]
Patch
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. Created attachment 353529 [details]
Patch
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. 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? 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. Created attachment 353701 [details]
Patch for landing
Comment on attachment 353701 [details] Patch for landing Clearing flags on attachment: 353701 Committed r237737: <https://trac.webkit.org/changeset/237737> All reviewed patches have been landed. Closing bug. |