WHATWG
2024-04-04
plaintext logs
prev next
source on github
06:09
<freddy>
Invites sent
10:25
<zcorpan>
Who remembers this gem? https://www.youtube.com/watch?v=zoInPRApZcc (the original site is no longer functional, and it was Flash - https://web.archive.org/web/20110415095516/http://www.xtranormal.com/watch/7991991/web-sockets-we-are-the-first )
10:29
<annevk>
Have you heard about the IndexDB? 😂
12:11
<peterv>
Luke Warlow: the PR for StringContext doesn't have any changes for distinguishability, but I wonder if that's ok. Strings and objects are distinguishable in general, but I'm not sure that that's going to work with StringContext annotated strings.
12:11
<peterv>
since that can take a trusted type object
12:13
<Luke Warlow>
Generally speaking we don't need overloading for the use case of StringContext, and I think that's why the PR has kinda skirted around that issue. Given StringContext has side effects I'm not sure how it could work to resolve the overloads
12:15
<peterv>
right, so we might need to make StringContext-annotated strings not distinguishable from object, or anything that object is not distinguishable from
12:15
<peterv>
this also affects unions iirc
12:17
<Luke Warlow>
The approach TT has taken for where it needs unions is to just use the Trusted type directly and handle the enforcement in the specific algorithm for that call site. So we also don't need to "support" that case. The only thing we do need to support is shadowing an IDL attribute on a subtype, I'm not sure how that works spec wise?
12:22
<peterv>
that just adds a property on the derived interface's interface prototype object
12:23
<peterv>
I assume the shadowing adds a StringContext annotation?
12:24
<peterv>
hmm, but the base interface also has a property for that attribute that would just accept a string
12:24
<peterv>
and you can call that property's setter directly
12:25
<Luke Warlow>
In this specific case that's fine as there's another fallback mechanism to protect against it
12:25
<peterv>
ok
12:25
<peterv>
that should just work as is I think (for attributes)
12:26
<Luke Warlow>
Yeah that was my impression too (it does in practice in my implementation). Just wondered if that came into the overloading stuff at all.
12:27
<peterv>
shouldn't, that's for operations
12:53
<peterv>
annevk: is the idea that none of the arguments that have an index >= distinguishing argument index can have a [StringContext] extended attribute on their type?
13:41
<annevk>
peterv: I hadn't really considered that yet. It seems weird that [StringContext] only works some of the time and the remainder you have to patch the specification algorithm directly. Why not just patch the specification algorithm in all cases then?
13:48
<peterv>
It almost feels like we should treat [StringContext]-annotated strings as a different string type from non-[StringContext]-annotated, that way all string types for index < distinguishing argument index must all either have [StringContext] or not
13:48
<peterv>
since they must have the same type
13:48
<Luke Warlow>
peterv: I hadn't really considered that yet. It seems weird that [StringContext] only works some of the time and the remainder you have to patch the specification algorithm directly. Why not just patch the specification algorithm in all cases then?
Because that's much more intrusive in specs and in implementations.
13:48
<peterv>
if they're in the same position in the argument list that is
13:48
<Luke Warlow>
With this attribute we can avoid touching most callsites and spec algorithms
13:50
<peterv>
but for index >= distinguishing argument index it's unclear to me what we want
13:51
<peterv>
where does that come up in practice?
13:52
<annevk>
I think it doesn't come up currently because it's sparsely used. Which makes me wonder how many call sites we are worried about here.
13:52
<annevk>
But it's also becoming clear that as a generic IDL utility there's quite a few issues with it which makes me wonder if it will ever see usage beyond Trusted Types.
13:52
<peterv>
hmm, can we cop out then and disallow that?
13:53
<annevk>
We could or we could not add this to IDL and instead have the various specification algorithms deal with it directly. As the more complicated cases are already doing.
13:56
<peterv>
so they would all take TrustedType or DOM/USVString?
13:57
<peterv>
a bit messy for attributes, since they'll only return a string ever
13:57
<annevk>
Yeah and the first step of their algorithm does something to turn that into an Infra string. Presumably that matches what we do for the more complicated cases Luke Warlow?
13:58
<annevk>
Yeah, I guess this brings up attributes really needing to be separated into setters and getters again.
13:58
<Luke Warlow>

Yeah pretty much, https://pr-preview.s3.amazonaws.com/lukewarlow/trusted-types/pull/484.html#the-textContent-idl-attribute shows a good example.

It uses a union (so we don't have to deal with nullable types with StringContext) and then step 1 is just to call the algorithm that handles converting to a string (with enforcement)

14:00
<Luke Warlow>
Fwiw at least in WebKit if you have an attribute with a union type the function can just return one of the types directly and it'll handle it further up. So at least the getters don't need to worry about variants
14:01
<Luke Warlow>
I think it doesn't come up currently because it's sparsely used. Which makes me wonder how many call sites we are worried about here.
I'm counting the callsites we'd have to change now
14:01
<annevk>
To me that doesn't seem so bad and preferable over this StringContext business. Especially if there's only a dozen or so callers that use StringContext currently. Or are there many more?
14:08
<Luke Warlow>
Element innerHTML
Element outerHTML
Element insertAdjacentHTML
Element setHTMLUnsafe

ShadowRoot innerHTML
ShadowRoot setHTMLUnsafe

Document parseHTMLUnsafe
Document parseFromString

Range createContextualFragment

HTMLIframeElement srcdoc

Window/Worker setTimeout
Window/Worker setInterval


HTMLScriptElement innerText
HTMLScriptElement src
HTMLScriptElement text

ServiceWorkerContainer register

WorkerGlobalScope importScripts
14:09
<Luke Warlow>
I think that's the full list (unless we find anymore missing ones 🙈), so just shy of 20.
14:11
<Ms2ger>
Where does HTMLScriptElement innerText come from?
14:11
<Luke Warlow>
https://pr-preview.s3.amazonaws.com/lukewarlow/trusted-types/pull/484.html#dom-htmlscriptelement-innertext This PR which partially fixes the TT specs script handling
14:14
<Luke Warlow>
To me that doesn't seem so bad and preferable over this StringContext business. Especially if there's only a dozen or so callers that use StringContext currently. Or are there many more?
My gut feeling is this attribute probably won't get used outside of TT. And that the changes to algorithms are probably unintrusive enough (exactly because they're the simple cases) that it's probably fine to just go with union IDL types everywhere. Obviously it will require changes to implementations, but its entirely an implementation detail (not an observable difference in behaviour) so it's not too much of a hassle?
14:15
<Ms2ger>
Implementations will need work anyway :)
14:16
<Luke Warlow>
Implementations will need work anyway :)
I mean yeah exactly, it's not even implemented in Gecko yet (I don't think), I can do the webkit change easily enough, so it's really Chrome that it would be extra work for but even then they'll have to make changes to their implementation in various places anyway.
14:18
<peterv>
I'm wondering, why doesn't Element setAttribute need to be changed too?
14:18
<Luke Warlow>
It uses a union type, the list above is just ones currently using the IDL attribute
14:18
<zcorpan>
Luke Warlow: https://source.chromium.org/search?q=(scripturlstring%20OR%20htmlstring%20OR%20scriptstring%20OR%20trustedtype)%20file:.idl&ss=chromium
14:19
<peterv>
It uses a union type, the list above is just ones currently using the IDL attribute
as in, it will use a union type?
14:19
<Ms2ger>
peterv: there's an "attribute change steps" override
14:19
<peterv>
sure, but WebIDL takes a DOMString
14:19
<Ms2ger>
Though that doesn't answer your question
14:19
<Luke Warlow>
It already does per the spec (that one is in a PR to DOM still). there's a few more complicated call sites already setup that way
14:20
<peterv>
ah, ok, so that change still needs to happen
14:20
<peterv>
I didn't see it in the TT spec as one of the changes that need to be done to other specs
14:21
<Luke Warlow>
I didn't see it in the TT spec as one of the changes that need to be done to other specs
Yeah that particular one is in an odd state, I should probably add a note to TT to point to the PRs
14:21
<peterv>
I'm fine with not having the extended attribute given that we have a mix now
14:22
<peterv>
the extended attribute is nice because it documents what happens, but if it's not used everywhere that isn't exactly true
14:22
<Luke Warlow>
All the DOM APIs (of which a several are changed), execCommand, SVGAnimatedString baseVal, HTMLScriptElement textContent all already use (or are planned to) union types (potentially others too)
14:23
<peterv>
well, the union also documents but it's more complicated to understand in that the end result is still a string value
14:24
<Luke Warlow>
well, the union also documents but it's more complicated to understand in that the end result is still a string value
Yeah I think ultimately TT complicates things no matter what form it's in. So it's probably fine?
14:26
<peterv>
in terms of implementation, the extended attribute might be easier for adding special treatment in the binding code itself (with the union you'd have to detect these special unions)
14:27
<Luke Warlow>
So I think the binding code would treat it as any other union and you'd just update the callsite directly
14:27
<peterv>
but I don't have implementation experience 🙂
14:27
<Luke Warlow>
Bit messier having to unpack a variant, but it's pretty fine, and we already have to do it in a number of places :)
14:28
<peterv>
hmm, I think I would advocate doing it in the generated code in Gecko 🙂
14:28
<peterv>
there's all kinds of complications with the exceptions etc that I'd rather centralize
14:29
<Luke Warlow>
Mirko was working on that recently and as I understand it the binding code is quite custom in gecko already, so probably something nice that can be done there.
14:34
<Luke Warlow>
Next steps are to close that PR out and update the spec to use unions instead? Should we wait to hear from Googlers?
14:42
<peterv>
I think we should wait a bit 🙂
16:07
<keithamus>
Is there a meeting happening? I requested to join but I don't know if I'm in the wrong one or what.
16:10
<annevk>
keithamus: my calendar says there's a meeting at 1AM Berlin time.
16:11
<keithamus>
Ah, odd. Mine says there's one during this hour. I guess that's in error, and I'll delete. Thanks annevk
16:12
<annevk>
Yeah, looks like it's in error: https://github.com/whatwg/html/issues/10229
23:15
<Domenic>
My gut feeling is this attribute probably won't get used outside of TT. And that the changes to algorithms are probably unintrusive enough (exactly because they're the simple cases) that it's probably fine to just go with union IDL types everywhere. Obviously it will require changes to implementations, but its entirely an implementation detail (not an observable difference in behaviour) so it's not too much of a hassle?
I think it's observable via exception order and other side effects during argument processing. (x, y, z) where right now doing TT conversions on y would throw an exception. Now we'd convert z before doing TT conversions.