Ads/Runtime: move getIntersectionChangeEntry out of Runtime and into Ads only #34133
Conversation
|
Hey @Jiaming-X! These files were changed: Hey @jeffkaufman! These files were changed: |
| * @return {!IntersectionChangeEntry} a change entry | ||
| */ | ||
| export function getIntersectionChangeEntry(element) { | ||
| const box = element.getLayoutBox(); |
There was a problem hiding this comment.
This is intended to get the bounding client rect of the ad iframe, not the amp-element. Are we sure the size is the same?
Also, this is an absolutely positioned (from the top-left corner of the <body>) box, and not a viewport-relative box, which is extremely important for ads (they want to know that they're in viewport, not 10m pixels down the page). We can accommodate that in getIntersectionChangeEntryHelper by subtracting the viewportBox's position.
samouri
force-pushed
the
nuclear
branch
8 times, most recently
from
c03badc to
4a806a9
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
summary
I have been attempting to remove all usages of
getIntersectionChangeEntryfrom the Runtime since last December (#31753). One last usage has been particularly tricky to remove because of being part of the ads render flow, and the requirement to ensure it doesn't degrade impressions.This PR opts to remove ads as a blocker, by simply moving all the same logic to
adsfolders and out ofcustom-element. Note: this creates a usage ofcustom-element.getLayoutBox, but we can deal with that in a future PR.changes
getIntersectionChangeEntrywithin intersection-observer-3p-host togetIntersectionChangeEntryHelper.getIntersectionChangeEntryfrom custom-element to intersection-observer-3p-host, and updates all the associated tests.Size changes: