w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
319 stars 55 forks source link

Specification review for fenced frames #838

Open domfarolino opened 1 year ago

domfarolino commented 1 year ago

こんにちは TAG-さん!

I'm requesting a TAG review of fenced frames.

Overview of proposal In a web that has its cookies and storage partitioned by top-frame site, there are occasions (such as Interest group based advertising or Conversion Lift Measurements) when it would be useful to display content from different partitions in the same page. This can only be allowed if the documents that contain data from different partitions are isolated from each other such that they're visually composed on the page, but unable to communicate with each other. Iframes do not suit this purpose since they have many communication channels with their embedding frame (e.g., postMessage, URLs, size attribute, name attribute, etc.). We propose fenced frames, a new element to embed documents on a page, that explicitly prevents communication between the embedder and the frame.

Delta Please note that previously we had an early-review TAG design review that had resolved positively: https://github.com/w3ctag/design-reviews/issues/735#issuecomment-1226822420. Since then, our spec has become considerably more fleshed out, and parts of our API surface have changed substantially, captured in https://github.com/WICG/fenced-frame/blob/master/explainer/fenced_frame_config.md that we discussed at last year's TPAC. More or less, we've removed the src attribute from the fenced frame element, in favor or manipulating the element with the new FencedFrameConfig interface, which can be returned from various APIs related to this proposal, such as Protected Audience and Shared Storage.

Details

Further details:

We'd prefer the TAG provide feedback as:

💬 leave review feedback as a comment in this issue and @-notify [GitHub usernames]

torgo commented 1 year ago

Hi @domfarolino - Some notes from our TAG breakout today: We're noting that we had previously reviewed an earlier version of this spec and given it a positive review. In fact, we had also specifically called it out as a positive development in our finding on Improving the Web Without Third Party Cookies.

We're concerned that the changes to this API include Shared Storage as a dependency. Please note that not only does this not have multi-stakeholder support, but important stakeholders have actively expressed opposition to it: e.g. see the Webkit Standards Position comment, by @annevk and the Mozilla Standards Position. We've also noted this as an issue in our ongoing review of Shared Storage. Can you please clarify – is there a dependency on Shared Storage? Does this dependency improve user experience or developer experience of this feature? What's your plan in the (very likely) case that Shared Storage is not implemented by other browsers?

We are actually working on a design principle that would state config objects should also accept plain object literals whenever possible. Would that guidance be helpful in your design process?

We appreciate the thorough job done on documenting this API. Looking through the use cases though, we're struggling to find the user need. We're noting the goal articulated in the main explainer but what's missing is a simple articulation of what benefit the user derives from the inclusion of this API in the platform - from the web user's point of view. We'd also like to see an explanation and code example of how the API is used in its most simple form.

rhiaro commented 1 year ago

Hi there. Just to reiterate @torgo's last point - I've found it challenging to review this across an 12 page, very detailed technical explainer. While this is great documentation, it isn't really in the spirit of the purpose that TAG explainers need to serve. I have the impression there is a lot of context shared between the people who are working on this day-to-day that I'm missing due to not having a lot of exposure to or a deep understanding of the work, so it's hard for me to know which parts to focus on.

Please could you provide a one-pager primer or overview, and take note of our tips on how to write effective explainers? This can of course link out to more detail, but the key points should be captured a single document.

A smaller point - FLEDGE, Turtledove and Protected Audience are used interchangeably throughout. I understand that these are all effectively the same thing? It would also be helpful if you could improve consistency here.

domfarolino commented 1 year ago

The fenced frames proposal does not depend on Shared Storage, no. Shared Storage in some part depends on fenced frames, but not the other way around.

domfarolino commented 1 year ago

I've filed https://github.com/WICG/shared-storage/issues/98 to note that any shared storage-specific APIs that appear in the fenced frames specification should be _instead specified in the shared storage specification as extensions of the fenced frames spec, to make the dependency very clear — that fenced frames as a feature can stand alone without shared storage.

We are actually https://github.com/w3ctag/design-principles/issues/11#issuecomment-1515585583 that would state config objects should also accept plain object literals whenever possible. Would that guidance be helpful in your design process?

I do think this guidance would be pretty useful in general, although we might be running into the "wherever possible" loophole a bit, since the value of a FencedFrameConfig object actually has to do with an internal token that is unique to the config and irreplicable on the web platform. The config is largely read-only, reflecting the pieces of information that a config generator API is allowed to expose to the web platform without compromising privacy, so being able to interchange this with a JS-created bag of properties might not be immediately possible for us without considering another mode of fenced frames which allows user/web-platform-created config objects (which we've indeed discussed in the past, and is on the table for future consideration).


Regarding the explainer feedback, I was really hopeful that https://github.com/WICG/fenced-frame/blob/master/explainer/README.md would be sufficient to review. I feel that it's not too much longer than a lot of explainers, indeed links out to many other documents as you recommended, and I think we capture "the key points in a single document".

rhiaro commented 1 year ago

I was really hopeful that https://github.com/WICG/fenced-frame/blob/master/explainer/README.md would be sufficient to review

Any chance you could add end-user needs and code examples?

When we discussed in our earlier call, we struggled to get a sense of the end-to-end experience of using the API as a developer, from the perspectives of the various parties involved. I think code examples (and possibly a diagram) would help with this.

The code example that is present includes: fencedframe.config = new FencedFrameConfig('demo_fenced_frame.html'); which - if I understand correctly from the spec and your comments above - is explicitly not how this API is supposed to be used?

torgo commented 1 year ago

The fenced frames proposal does not depend on Shared Storage, no.

Thanks @domfarolino. I think that really needs to be clarified in the explainer because from our read of the explainer (and apparently from Anne's read as well) it looked like a dependency.

Also: I appreciate the work that's gone into the explainer(s) already and sorry if this seems like make-work. As we've noted in our explainer explainer we're really trying to get people to start from end-user need. This derives from the architectural principle of putting users' needs first.

torgo commented 12 months ago

@rhiaro wrote:

A smaller point - FLEDGE, Turtledove and Protected Audience are used interchangeably throughout. I understand that these are all effectively the same thing? It would also be helpful if you could improve consistency here.

@domfarolino can you clarify this? Is there a single name we should be using to refer to this?

domfarolino commented 12 months ago

Yeah, we're slowly trying to update everything to use Protected Audience instead of the other two names (including renaming repositories when necessary, etc.) The naming around these things has indeed gotten unnecessarily confusing, sorry! We'll be making changes across the Privacy Sandbox proposals for consistency.

hadleybeeman commented 11 months ago

Hi all. We are looking at this at our W3CTAG f2f. We had a long discussion about how the shape of this, changing the relationship between an iFrame and its embedding page — it must not be unique to the advertising use cases you've listed.

We brainstormed along the lines of a site presenting user-generated content in an iFrame, and the payments processes. Have you explored use cases out side the ones you're citing? And if so, what overlaps are you finding?

shivanigithub commented 11 months ago

Hi all. We are looking at this at our W3CTAG f2f. We had a long discussion about how the shape of this, changing the relationship between an iFrame and its embedding page — it must not be unique to the advertising use cases you've listed.

We brainstormed along the lines of a site presenting user-generated content in an iFrame, and the payments processes. Have you explored use cases out side the ones you're citing? And if so, what overlaps are you finding?

Totally agree that the idea of fenced frames is not unique to the use cases supported thus far. We are actively also working towards supporting the personalized payment button use case as is being discussed on this issue and mentioned in our earlier TAG review: comment and support from the ecosystem. We are hoping to update our explainer/spec as we make more progress on the design for the same.

shivanigithub commented 10 months ago

FYI, linking a few recent updates to the spec for new features:

Send Automatic Beacons Once https://github.com/WICG/fenced-frame/pull/109

Serializable Fenced Frames Configs https://github.com/WICG/fenced-frame/pull/111

Creative Macros in Fenced Frames Ads Reporting (FFAR) Protected Audience: https://github.com/WICG/turtledove/pull/762/files Fenced Frames: https://github.com/WICG/fenced-frame/pull/113

blu25 commented 8 months ago

There are a few more recent updates to the spec for new features:

Send Automatic Beacons to Every Registered Destination: WICG/fenced-frame#122 WICG/fenced-frame#129

Ad Size Macro Substitution WICG/turtledove#801

shivanigithub commented 8 months ago

There are a few more recent updates to the spec for new features:

Enable Leaving Ad Interest Groups from Urn iFrames and Ad Component Frames https://github.com/WICG/turtledove/pull/880

Introduce reserved.top_navigation_start/commit https://github.com/WICG/fenced-frame/pull/130

blu25 commented 6 months ago

There is a recent update to the spec for a new feature:

Allow Cross-Origin Subframes to Send Automatic Beacons https://github.com/WICG/fenced-frame/pull/133

rhiaro commented 5 months ago

Hi folks -

We're noting that there have been many spec changes since our last comment but no changes to the explainer. Can you please explain what has changed since our previous review and a short explanation of why you made these changes?

We appreciate that you updated this thread with links each time you make updates to the spec, however these would also be better as updates to the explainer. It's good for an explainer to be a living document.

In general, we're concerned about the direction this is headed since our last review. Concepts such as "exfiltration budget" which seem to be at odds with the stated goals of the fenced frames proposal itself?

We've previously asked for use cases and user needs and we haven't really seen anything back. These should be added to the explainer as well.

shivanigithub commented 5 months ago

Hi folks -

We're noting that there have been many spec changes since our last comment but no changes to the explainer. Can you please explain what has changed since our previous review and a short explanation of why you made these changes?

We appreciate that you updated this thread with links each time you make updates to the spec, however these would also be better as updates to the explainer. It's good for an explainer to be a living document.

In general, we're concerned about the direction this is headed since our last review. Concepts such as "exfiltration budget" which seem to be at odds with the stated goals of the fenced frames proposal itself?

We've previously asked for use cases and user needs and we haven't really seen anything back. These should be added to the explainer as well.

Responding to explainer updates below but I had a quick clarification before that, I don't see "exfiltration budget" in any of the updates above, could that comment be referring to another TAG review, perhaps? Similarly for the comment about use cases, the use cases are actually defined in the fenced frames explainer - is that comment for another TAG review too?

Sure, we would be happy to link the explainer updates corresponding to the spec updates linked above. Since most of these updates are in the Protected Audience + Fenced Frames reporting space, most of those explainer updates are in the Protected Audience repo and we can list them here in a follow up comment.

rhiaro commented 5 months ago

Hi @shivanigithub thanks for getting back so quickly.

When beginning our re-review of this earlier we found references to exfiltration budget in the specification itself. We didn't recall this concept from our review in #735.

The use cases we see in the explainer are not from a user needs perspective as far as we can tell. We'd appreciate a perspective on how these features impact end users, developers, and other parties who may be involved (eg. advertisers). What are the problems each set of users is facing? How do the proposed features solve them? What trade-offs are there?

The questions in this comment we left in July still stand as far as I can tell.

gtanzer commented 5 months ago

The "exfiltration budget" field is only used by the Shared Storage spec (will go here once they resolve the reference: https://wicg.github.io/shared-storage/#issue-7ff511a8)

When the fenced frame config is not being created for Shared Storage, "exfiltration budget metadata" is null. Or if Shared Storage isn't supported, the field does not need to be implemented. I'll add a line to that effect in the spec.

shivanigithub commented 2 months ago

[Catching up on this thread now as I was unexpectedly out sick for a long while. Thanks for your patience, the fenced frames team is in the process of responding on the remaining open questions here.]

blu25 commented 2 months ago

We're noting that there have been many spec changes since our last comment but no changes to the explainer. Can you please explain what has changed since our previous review and a short explanation of why you made these changes?

Below is the list of all the recent spec changes we've made, along with links to their explainer updates + either links to the GH issues that prompted it or explanations for why we made the changes.

Send Automatic Beacons Once Spec: WICG/fenced-frame#109 Explainer: WICG/turtledove#718 Background: WICG/turtledove/issues/743 (originally raised by adtech partners but I wrote the issue for tracking purposes)

Serializable Fenced Frames Configs Spec: WICG/fenced-frame#111 Explainer: N/a Background: Supports a use case that involves running an ad auction in a subframe and then postMessaging it to a parent frame to create the actual fenced frame.

Creative Macros in Fenced Frames Ads Reporting (FFAR) Spec: Protected Audience: WICG/turtledove#762 Spec: Fenced Frames: WICG/fenced-frame#113 Explainer: WICG/turtledove#763 Background: WICG/turtledove/issues/477

Send Automatic Beacons to Every Registered Destination: Spec: WICG/fenced-frame#122 Spec: WICG/fenced-frame#129 Explainer: WICG/turtledove#808 Background: WICG/turtledove/issues/826

Ad Size Macro Substitution Spec: WICG/turtledove#801 Explainer: WICG/turtledove#417 Background: WICG/turtledove/issues/312

Enable Leaving Ad Interest Groups from Urn iFrames and Ad Component Frames Spec: WICG/turtledove#880 Explainer: WICG/turtledove#879 Background: Expanding functionality since there wasn't any reason to disallow this in component frames/URN iframes.

Introduce reserved.top_navigation_start/commit Spec: WICG/fenced-frame#130 Explainer: WICG/turtledove#885 Background: WICG/turtledove/issues/822 (originally raised by adtech partners but I wrote the issue for tracking purposes)

Allow Cross-Origin Subframes to Send Automatic Beacons Spec: WICG/fenced-frame#133 Explainer: WICG/turtledove#904 Background: WICG/turtledove/issues/877

torgo commented 1 month ago

Hi @blu25 - we are just reviewing it today. We appreciate you taking the time to collate the information you've provided. However, it's difficult for us to parse the list - which is a lot of links to PRs where you need a lot of context to understand how they fit in, context that we lack. This is one reason we ask for explainers with reviews - which provide that context.

I'd also like to point out that we've returned a negative review on Protected Audience. So if there is a hard dependency now between this work and Protected Audience then it's unlikely that this will get a positive review from TAG.

Even so, we strongly recommend that you write an explainer which talks through this proposal, starting from user needs - as documented in our explainer guide.

shivanigithub commented 1 month ago

Hi @blu25 - we are just reviewing it today. We appreciate you taking the time to collate the information you've provided. However, it's difficult for us to parse the list - which is a lot of links to PRs where you need a lot of context to understand how they fit in, context that we lack. This is one reason we ask for explainers with reviews - which provide that context.

A majority of the changes listed in this list are part of the “Fenced Frames Ads Reporting (FFAR)” infrastructure that is explained in detail in this explainer: https://github.com/WICG/turtledove/blob/main/Fenced_Frames_Ads_Reporting.md which is a short to medium term solution provided for Protected Audience adopters to be able to report on ad impressions within a fenced frame. Since some of the API surfaces are invoked from the fenced frame, they are part of the fenced frames spec but FFAR is ultimately a Protected Audience solution and is not core to fenced frames functionality.

Given your stated objection to Protected Audience, we ask that you instead consider only the following change from the above list that impacts fenced frames in general: Serializable Fenced Frames Configs.

I'd also like to point out that we've returned a negative review on Protected Audience. So if there is a hard dependency now between this work and Protected Audience then it's unlikely that this will get a positive review from TAG.

Even so, we strongly recommend that you write an explainer which talks through this proposal, starting from user needs - as documented in our explainer guide.

To add to above, fenced frames do not have a dependency on Protected Audience and are a generic element providing stronger isolation between the embedder and the frame, such that they are visually composed on the same page but cannot join cross-site identifiers. As the TAG had requested above, the updated use cases document is now published here. As an example of non-Protected Audience use case that fenced frames intend to support, as mentioned in the earlier comment, we are in the process of adding functionality for local unpartitioned data access which will support use cases like personalized payment buttons. That functionality is described in this explainer and the spec is currently in progress. Since this is a major update, we are happy to open a new TAG request specifically for this when the spec is more ready, or add to this request, as the TAG reviewers may see fit.

shivanigithub commented 10 hours ago

To add to above, fenced frames do not have a dependency on Protected Audience and are a generic element providing stronger isolation between the embedder and the frame, such that they are visually composed on the same page but cannot join cross-site identifiers. As the TAG had requested above, the updated use cases document is now published here. As an example of non-Protected Audience use case that fenced frames intend to support, as mentioned in the earlier comment, we are in the process of adding functionality for local unpartitioned data access which will support use cases like personalized payment buttons. That functionality is described in this explainer and the spec is currently in progress. Since this is a major update, we are happy to open a new TAG request specifically for this when the spec is more ready, or add to this request, as the TAG reviewers may see fit.

I created an early design TAG review for this new functionality described above: https://github.com/w3ctag/design-reviews/issues/975