search

WICG / fenced-frame

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

Add is ad component field to fenced frame config. #115

Closed xiaochen-z closed 8 months ago

xiaochen-z commented 10 months ago

This is the first step to spec the ad component reportEvent change.

Preview | Diff

domfarolino commented 10 months ago

How will this be used? I'm just trying to get a little context to review this, by understanding how it'll need to look in the future?

the ad component reportEvent change.

... is a little vague.

xiaochen-z commented 10 months ago

How will this be used? I'm just trying to get a little context to review this, by understanding how it'll need to look in the future?

the ad component reportEvent change.

... is a little vague.

For ad component, event reporting works different than the main ad. This boolean field allows related event reporting algorithms to know whether this ad is an ad component or not. Please see the explainer for ad component event reporting.

domfarolino commented 10 months ago

OK, could you include some of that context directly in the PR as a non-normative note, because that context is very useful? This seems to be the first time "component ads" are really referenced in the spec, so at the very least linking to that explainer content from the PR would be wonderful.

xiaochen-z commented 10 months ago

Thanks. I've added a note on ad components and the special handling on event reporting.

gtanzer commented 10 months ago

I'm not sure that this field is necessary in spec. I think it's used in the implementation only because the renderer can't access the entire frame tree with ancestors' fenced frame properties. But I might be misremembering.

domfarolino commented 10 months ago

I'm not sure that this field is necessary in spec. I think it's used in the implementation only because the renderer can't access the entire frame tree with ancestors' fenced frame properties. But I might be misremembering.

Should we abandon this or...? I'll trust people more familiar with the implementation these days to make the call.

xiaochen-z commented 10 months ago

This field is used at browser to identify an ad component frame. If we do not mention this field in the spec, is it sufficient to provide a definition of ad component and then describe the algorithm like "if this is an ad component, then ..."?

gtanzer commented 10 months ago

Actually maybe "is ad component" is necessary because you can (maybe not right now, but at some point) created nested fenced frames with different constructors, and you want to treat the actual ad component frames differently...

domfarolino commented 10 months ago

So shall we land this @gtanzer?

gtanzer commented 10 months ago

Sgtm, though maybe "is ad component" should have a default value of false.

xiaochen-z commented 9 months ago

Added default values.

domfarolino commented 9 months ago

Still look good @gtanzer ?

gtanzer commented 8 months ago

still lgtm