WICG / fenced-frame

Proposal for a strong boundary between a page and its embedded content
https://wicg.github.io/fenced-frame/
Other
127 stars 31 forks source link

Spec reserved.top_navigation_start and reserved.top_navigation_commit #130

Closed blu25 closed 11 months ago

blu25 commented 11 months ago

This PR does a few things to allow the new automatic beacon types:


Preview | Diff

michal-kalisz commented 11 months ago

This change appears to be very valuable. However, it breaks backward compatibility (discontinues support for the automatic beacon event type: reserved.top_navigation). When the change will be merged and available in chrome, some users will have an older version of the browser (requiring reserved.top_navigation), and some will have a newer one (supporting reserved.topnavigation(start|commit)).

Tracking clicks in our current tests is a critical component. Therefore, we are concerned that the proposed change should not affect either our tests or our partners' (SSP) tests.

How can we prepare for this? Would registering 3 automatic beacons: reserved.top_navigation, reserved.top_navigation_start, reserved.top_navigation_commit and then calling window.fence.setReportEventDataForAutomaticBeacons for 3 different event type in the ad be a solution?

Something like: In report win / report result:

function reportWin() {
    registerAdBeacon({
        "reserved.top_navigation": "URL_for_commit",
        "reserved.top_navigation_start": "URL_for_start",
        "reserved.top_navigation_commit": "URL_for_commit"
    })
}

In ad:

window.fence.setReportEventDataForAutomaticBeacons({
    eventType: 'reserved.top_navigation',
    destination: ['buyer', 'direct-seller']
});
window.fence.setReportEventDataForAutomaticBeacons({
    eventType: 'reserved.top_navigation_start',
    destination: ['buyer', 'direct-seller']
});
window.fence.setReportEventDataForAutomaticBeacons({
    eventType: 'reserved.top_navigation_commit',
    destination: ['buyer', 'direct-seller']
});

Or do you suggest something else?

blu25 commented 11 months ago

To clarify, reserved.top_navigation will continue to work in new versions of Chrome for the time being.

Our current plan is to remove reserved.top_navigation functionality at some point in the future once enough partners have switched over to reserved.top_navigation_commit. The initial version of this spec patch reflects what we want the eventual end-state to be. However, I can't commit to any timeline for actually removing reserved.top_navigation. In the meantime, and to avoid confusion about what is being supported on the Chrome side, I'll add it back to the spec with an advisement that it will be deprecated.

In terms of preparing for this change, what you described would be the preferred way of handling this. Once enough people are on the newer version of Chrome, you can remove the reserved.top_navigation part of your code.

dmdabbs commented 11 months ago

I also notice that this PR updates only the spec. Will explainers get updated as well?

blu25 commented 11 months ago

Yes. That's in a separate repository, and I have a PR up for that as well.