Spec deprecatedURNtoURL #161
Conversation
Addresses WICG#160
|
LGTM |
|
LGTM % fyi |
|
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. |
domfarolino
left a comment
There was a problem hiding this comment.
Pretty much lgtm; nothing major here.
Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
domfarolino
left a comment
There was a problem hiding this comment.
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
spec.bs
Outdated
|
|
||
| 1. Otherwise, set |urn| to |urnOrConfig|'s [=fencedframeconfig/urn=]. | ||
|
|
||
| 1. If |urn| is <span class=XXX>TODO invalid</span>, then [=exception/throw=] a {{TypeError}}. |
There was a problem hiding this comment.
#161 (comment) got marked outdated, so I guess I'll re-file this comment as a stand-in for it.
There was a problem hiding this comment.
I proposed some new words here, WDYT?
There was a problem hiding this comment.
I think that sounds right? Although it would be inelegant to have to write that i.e., each time we reference the concept of an invalid URN. But I suppose this is an incremental improvement so that's fine.
I feel like stuff that only relates to URNs, like |
|
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: |
spec.bs
Outdated
|
|
||
| 1. Otherwise, set |urn| to |urnOrConfig|'s [=fencedframeconfig/urn=]. | ||
|
|
||
| 1. If |urn| is <span class=XXX>TODO invalid</span>, then [=exception/throw=] a {{TypeError}}. |
There was a problem hiding this comment.
I think that sounds right? Although it would be inelegant to have to write that i.e., each time we reference the concept of an invalid URN. But I suppose this is an incremental improvement so that's fine.
domfarolino
left a comment
There was a problem hiding this comment.
(Meant to "approve" with nits, whoops)
Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
Addresses #160
Preview | Diff