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

Spec deprecatedURNtoURL #161

Closed JensenPaul closed 1 month ago

JensenPaul commented 1 month ago

Addresses #160

Preview | Diff

qingxinwu commented 1 month ago

LGTM

gtanzer commented 1 month ago

LGTM % fyi

domfarolino commented 1 month ago

I saw this in my inbox but I noticed I was not added as a reviewer. Should I take a look? Feel free to add me as a reviewer and I will take a look after I'm back from vacation.

JensenPaul commented 1 month ago

Btw what is the motivation to include this API in this spec, instead of the PA spec? Impl-wise, it looks like it was included closer to the PA stuff: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/ad_auction/navigator_auction.idl;l=41;drc=ac83a5a2d3c04763d86ce16d92f3904cc9566d3a;bpv=1;bpt=0

I feel like stuff that only relates to URNs, like deprecatedURNtoURL(), should be in the FF spec, like deprecatedReplaceInURN() already is. The use of URNs (and then FFConfig) seems tied to seeding FFs, and could be used by specs other than PA.

domfarolino commented 1 month ago

Ah darn it looks there are conflicts now that the other PR merged. Would you be able to sort them out please?

JensenPaul commented 1 month ago

Ah darn it looks there are conflicts now that the other PR merged. Would you be able to sort them out please?

I fixed the merge conflict. Now it looks like the build is failing on an unrelated line: LINE 1724: Couldn't find WPT test 'fenced-frame/fence-report-event-cross-origin.https.html' - did you misspell something? Looks related to https://chromium-review.googlesource.com/c/chromium/src/+/5544804 So I updated the WPT file name.