07:37
<annevk>
Hmm, still no review
08:22
<annevk>
jgraham: https://github.com/w3c/web-platform-tests/pull/2283
08:23
<annevk>
jgraham: changes seem correct
08:52
<jgraham>
annevk: You don't have the ability to merge?
08:52
<annevk>
jgraham: no
08:53
<jgraham>
annevk: Fixed that, thanks for the review
10:24
<zcorpan>
annevk: Domenic: do you have an objection to changing the return value for add() in https://www.w3.org/Bugs/Public/show_bug.cgi?id=29061 ?
10:29
Ms2ger
is not convinced
10:33
<zcorpan>
Ms2ger: why not?
10:48
<annevk>
zcorpan: the known tokens approach seems better
10:49
<yoav>
annevk: pointer to that approach?
10:49
<annevk>
yoav: it's explained in the issue
10:50
<yoav>
So the approach outlined in comment 8?
10:51
<zcorpan>
annevk: it makes legacy browsers go into the "supported" branch
10:54
<zcorpan>
iframe.sandbox.add('foo'); /* ignore */ if (iframe.sandbox.has('foo')) { /* supported, or legacy browser */ }
10:56
<zcorpan>
i don't mind doing what comment 8 says (sans throwing), but it seems nice to have legacy browsers go into the unsupported branch, which is what boolean return value for add() does
10:56
<zcorpan>
i.e. if (iframe.sandbox.add('foo')) { /* supported */ } else { /* unsupported or legacy browser */ }
10:58
<yoav>
+1 to the latter
11:05
<yoav>
zcorpan: the `toggle()` case https://dom.spec.whatwg.org/#dom-domtokenlist-toggle seems more complicated to handle then `add()`, since it already returns false in various cases
11:05
<yoav>
we could also add that it returns false when trying to add an invalid token
11:07
<yoav>
So add to https://dom.spec.whatwg.org/#dom-domtokenlist-toggle before step 4.2 "if token is invalid, return false"
11:08
<zcorpan>
so the semantic right now for toggle is to return false if the token was removed, and return true if it's added, afaict
11:09
<zcorpan>
maybe we should think about what remove() should do, and then toggle() becomes clearer what it should do
11:11
<zcorpan>
i think remove/contains don't need to validate. and toggle should then only validate just before adding, not for removing
11:12
<zcorpan>
but that is mostly arm-chair reasoning on my part. would be nice to walk though a real example that uses toggle
11:15
<yoav>
zcorpan: FWIW, I agree with your arm-chair reasoning
11:15
<yoav>
since if an invalid token would never get in, there's no point in validating on the way out
11:16
<yoav>
and for feature detection purposes, a single method is enough
11:17
<Ms2ger>
I think it would be really weird if add() only worked on some strings
11:17
<Ms2ger>
Btw, how does this scale to classList?
11:18
<yoav>
classList won't implement a validation algorithm, which means everything is valid
11:21
<zcorpan>
yoav: invalid things could get in from the html parser or .rel = 'foo bar'; or setAttribute and so on
11:22
<yoav>
zcorpan: good point
11:23
<yoav>
still, if all we need is feature detection (which is the use case at hand for both iframe and link), add/toggle would be enough
11:24
<yoav>
also, being unable to remove invalid values added elsewhere would be weird
11:25
<Ms2ger>
Note that toggle already returns a boolean
11:28
<zcorpan>
Ms2ger: right
11:31
zcorpan
only finds test cases using toggle()
11:33
<zcorpan>
so back at the armchair, the boolean return value would be consistent with current toggle: if the token is added, return true, otherwise return false
11:33
<zcorpan>
and we only add the token in add() and toggle() if it's valid
11:34
<Ms2ger>
Why?
11:35
<Ms2ger>
I think that would be very surprising, and I'm not convinced it
11:35
<Ms2ger>
'd be compatible
11:35
<zcorpan>
so that you can check if a keyword is supported, and have current browsers fall into the "unsupported" branch
11:35
<Ms2ger>
That's what the return value is for
11:36
<Ms2ger>
I don't see a use case for dropping the token on the floor too
11:36
<zcorpan>
Ms2ger: oh, do you just find the validation thing surprising?
11:36
<zcorpan>
ok
11:36
<zcorpan>
i can live with that, but anne and dominic argued for that, to be consistent with "limited to only known values"
11:38
<Ms2ger>
Doesn't that still setAttribute()?
11:39
Ms2ger
pulls up the spec
11:39
zcorpan
parse error
11:39
<Ms2ger>
"and on setting, the content attribute must be set to the specified new value."
11:40
<Ms2ger>
So what I suggested is more in line with "limited to only known values"
11:40
<zcorpan>
....yes, yes you're right
11:54
<yoav>
Ms2ger: makes sense to me
11:55
<Ms2ger>
Always nice if existing practice agrees with you :)
12:49
<zcorpan>
should DOMTokenList be ascii-case-insensitive for <link rel> and <iframe sandbox>?
12:58
<Ms2ger>
And classList in quirks?
13:07
<annevk>
Ms2ger: not really sure if that is more in line
13:08
<annevk>
Ms2ger: perhaps if you're only comparing the setter and add()
13:09
<Ms2ger>
These seem the relevant things to compare
13:11
<annevk>
Ms2ger: so would then compare the getter with the iterator?
13:12
<annevk>
Ms2ger: it's a rather incomplete model
13:13
<Ms2ger>
Sure
13:13
<Ms2ger>
But there doesn't seem to be much precedent for dropping changes to content attributes
13:14
<annevk>
It just wouldn't propagate to the content attribute
13:14
<annevk>
There isn't much precedent for this kind of API either way
13:21
<yoav>
Took a stab at adding the hooks: https://github.com/whatwg/dom/pull/103
13:54
<annevk>
yoav: I think only doing something with add() is really weird
13:55
<yoav>
annevk: do you think we need to add something similar to toggle()?
13:55
<yoav>
or also remove and contains?
13:56
<yoav>
I can think of a way to easily add a false return for toggle, less so for remove and contains
13:57
<annevk>
yoav: the other problem I see is that this doesn't deal with case-sensitivity
14:00
<yoav>
annevk: yeah, I saw the discussion here after I sent the PR. I'll add case sensitivity to the validation algo
14:01
<annevk>
But in general this design just seems rather weird
14:02
<yoav>
annevk: something in particular that bothers you, outside of the issues you already mentioned?
14:03
yoav
gotta go now, but will check logs in a short while
14:03
<annevk>
yoav: I can't quite put my finger on it :-/
14:04
<annevk>
Something with having this new internal slot and only add() using it for a return value that indicates support and not wether something got added or not
14:04
<annevk>
It just seems rather confusing
14:05
<annevk>
And by not tying that "supported list" to values that can actually be added there's a fair chance implementations will not keep the two in lockstep
14:06
<annevk>
Which the getter/setter design of "known values" does not have in the slightest
14:43
<annevk>
wanderview: good morning!
14:43
<annevk>
wanderview: and also: https://github.com/whatwg/fetch/pull/146 😊
14:45
<wanderview>
annevk: looking
14:53
<annevk>
wanderview: ta
14:54
<wanderview>
np... kind of nit picky I guess
14:54
<annevk>
wanderview: nooo I think you found a problem
14:55
<wanderview>
annevk: did you guys ever get to discuss foreign fetch at tpac?
14:57
<yoav>
annevk: ms2ger raised concerns that changing the behavior of "add()" is likely to result in web compat issues https://www.w3.org/Bugs/Public/show_bug.cgi?id=29061#c17
14:57
<yoav>
I'm fine with it either way
14:58
<annevk>
wanderview: we did briefly
14:58
<Ms2ger>
Not sure about "likely", just "more likely"
14:58
<annevk>
wanderview: but not in detail
15:37
<annevk>
hsivonen: https://twitter.com/ken_lunde/status/661351862155669506
15:38
<wanderview>
annevk: so basically the f2f was still focused on "v1" things?
15:38
<annevk>
wanderview: yes
16:03
<hsivonen>
annevk: :-(
17:05
gsnedders
wonders how to sanely UA sniff IE9–11
17:06
<gsnedders>
Purely visual bug, so no way to check it. <_<