Change DOMTokenList's add() return value.

[yoavweiss]

As suggested by @bzbarsky:

Change the return value of add() to have three states: "supported", "not
supported" and "no supported tokens".

The "validation steps" algorithm return values also change accordingly.

/cc @foolip @zcorpan @annevk

[zcorpan]

These will all be truthy. Should we change "not supported" to "" (like canPlayType)?

[zcorpan]

Need to change the IDL to DOMString

[domenic]

It should be an enum, not a DOMString, right?

[bzbarsky]

These will all be truthy.

Yes, the basic idea, imo, is that falsy means API not supported, truthy means supported and then you can check the value.

[zcorpan]

That seems like suboptimal ergonomics on the long term. I would rather have people check for !== undefined in the short term, and do simple truthy check on the long term, I think.

[foolip]

@zcorpan, which three strings would you have? With two non-empty string, not both of them can mean supported, so I'm not quite seeing how to use this as a boolean even in the long run...

[foolip]

Oh, wait, I guess you mean in the very distant future when one can assume that all browsers will support tokens so that conflating "no supported tokens" with "supported" wouldn't be a concern?

[zcorpan]

Yeah. "No supported tokens" means n/a, e.g. for classList. No point in checking that value, but maybe nice to differentiate those two cases?

[bzbarsky]

Which long term are you thinking of? Note that one issue with the boolean setup that was raised is what happens if a list with no supported tokens changes to one with supported tokens. Suddenly you have no way to trust return values until everyone updates, and you can't even do the !== undefined trick. Hence the desire for a tristate return value to start with.

But also, what experience we have with the !== undefined thing shows that people won't get it right. At least the one person I saw try to handle it so far got it wrong, and that was a browser engineer...

[zcorpan]

I was thinking of when you don't have to worry about browsers that return undefined.

I don't mind having three states here, I just think it would be nice to be able to do a truthy check. I can buy that people will get comparisons wrong between empty string and undefined, but that is a temporary problem, right?

[bzbarsky]

I was thinking of when you don't have to worry about browsers that return undefined.

How does that address the problem of supported token sets being added to something that doesn't have them?

I guess we could have a falsy return value for "not supported" and two different truthy ones for "supported" and "no supported set", but that just means that code written to do the truthy check will break. If we think that at any point in the future things will transition from no supported set to having one, we really need to make it required to do an explicit tristate check for consumers.

but that is a temporary problem, right?

Only if we think nothing else will ever transition from no supported set to supported set.

[domenic]

I think we can assume that. All DOMTokenLists in all specs have been assessed and a supported set added where appropriate (or at least that was the intent). Future DOMTokenLists should consider this question immediately, not some time after they are introduced. It seems OK.

[zcorpan]

OK I see what you are saying. I suppose we could make the classList case also be falsy, e.g. continue to return undefined. But I also don't expect people to check the return value for classList, and I don't expect classList to get a list of supported keywords. But certainly it is difficult to predict the future.

I would be fine with changing back classList to return undefined, and either keep the other values as boolean, or use "" and "supported".

[annevk]

I think we can assume that.

I don't think so, so far HTML doesn't say anything about <a>, whereas that supports noreferrer and nowadays noopener as values for its rel attribute.

As for the enum values, they need to either not use spaces or use hyphens per https://w3ctag.github.io/design-principles/#casing-rules.

[yoavweiss]

I agree with @annevk that <a> may get supported tokens in the future, if the need to feature detect support for various rel attributes arises.

Are we all in agreement that a (truthy) tri-state is needed? Is there a preference with regard to strings vs. enums?

[zcorpan]

Hold on. So in #103 (comment) it is pointed out that it's likely not going to be Web-compatible to reject invalid values for <a>.relList.add(). If we want this for <a> as well, maybe we should revert the rejecting invalid values, and enable it for <a> now. (And <area>?)

[foolip]

@zcorpan

I was also a bit surprised to discover that in #111 (comment)

I think we should change the return value only and not change the other behavior.

@yoavweiss

A tri-state return value seems necessary, yes. Enums are string, so the question is only what the strings should be.

[domenic]

Per IRC @zcorpan and I have a tentative new plan.

• Remove validation from .add() and give up on the idea that we're trying to sync the token list to actually supported values (and thus help spec authors keep the list of supported keywords and the actual support in sync). Hope test coverage carries the day here.
• Remove .add()'s return value
• Add .supports(token) with a boolean return value
  • We think we can get away with the boolean return value instead of a tri-state enum since .supports() is a new method.
• Put to bed any worries about whether we'll in the future upgrade a DOMTokenList to have supported values by, once and for all, deciding on which DOMTokenLists receive this upgrade, and which don't. So that means if we're worried about <a rel>, we need to figure out what the supported <a rel> tokens are right now, and not let vague worries about it shape API design.
• .supports() means "has an impact on browser processing," not "is specified". So e.g. a.relList.supports("tag") is false, despite "tag" being a specified rel. Whereas a.relList.supports("noopener") will be true, for UAs that implement it.
• Open question: what to do about DOMTokenLists that don't have a validation algorithm or a set of supported values? Like classList?
  • (A) split into two classes, e.g. DOMTokenList and DOMSupportedTokenList, with only the latter having .supports(). Use each appropriately.
  • (B) have .supports() always return true. "A classList supports all classes."
  • (C) have .supports() always return false. "specific classes do not have impact on browser processing."

[phistuck]

Looks good. I am in favor of C (which contradicts "Put to bed any worries about whether...").

[yoavweiss]

So I went over the HTML spec and picked up all the relevant attributes.

DOMTokenList:

classList
a.relList
area.relList
link.relList

DOMSettableTokenList:

output.htmlFor
tablecell.headers
element.dropzone
link.sizes
a.ping
iframe.sandbox
area.ping

As far as I can tell we can split them into:

• Need "supported tokens" - link.relList, iframe.sandbox, element.dropzone, a.relList, area.relList
• Will never ever need "supported tokens" - classList, link.sizes, tablecell.headers, output.htmlFor, a.ping, area.ping

Does that make sense?

[annevk]

Yeah that makes sense. Note also #119 about simplifying the dual class setup to just a single class.

[zcorpan]

@phistuck

I am in favor of C (which contradicts "Put to bed any worries about whether...").

Why?

I think i prefer A because it leaves classList alone and makes it obvious how to feature-check for something moving from like-classList to like-relList, should that happen.

[phistuck]

@zcorpan - oops. Sorry. I must have been blind or too tired when I wrote it. I obviously meant A (which indeed contradicts that quote). Separate interfaces for an easy and accurate feature detection.

[phistuck]

@zcorpan - though, if A is chosen, the DOMSettableTokenList removal would be funny, because this introduces yet another subclass.
If we want a one-solution-fits-everything approach, then perhaps have two additional methods -

interface DOMTokenList {
    boolean supports(string value);
    readonly boolean hasSupportedTokens;
    ...
}

The almighty DOMTokenList.

[zcorpan]

The goal is not one-solution-fits-everything. Separate interfaces for supports and lack of supports makes sense, but there's no reason to have separate interfaces for value and lack of value. Removing DOMSettableTokenList means we have 2 interfaces after this instead of 4.

[foolip]

I'm not so sure, it seems like a bit of overkill to introduce a new interface for this, is "Put to bed any worries about whether we'll in the future upgrade a DOMTokenList to have supported values by, once and for all, deciding on which DOMTokenLists receive this upgrade, and which don't" not enough?

[phistuck]

We cannot pretend to predict the future, so some kind of a feature detection for having supported tokens is needed. Either a new interface, or a hasSupportedTokens attribute and a supports method on DOMTokenList (I think I prefer the attribute and method approach). Another way is a supportedTokens array and no method (that was dismissed earlier due to the fear of falling out of synchronization).

[domenic]

We cannot pretend to predict the future, so some kind of a feature detection for having supported tokens is needed. Either a new interface, or a hasSupportedTokens attribute and a supports method on DOMTokenList (I think I prefer the attribute and method approach).

This just seems wrong. No feature detection for having supported tokens is needed. We can easily decide which DOMTokenLists have supported tokens once and for all and never have to worry about "feature detection for having supported tokens".

[phistuck]

I prefer the relList.supportedTokens && relList.supportedTokens.includes("noopener") array/list/whatever approach, really.

[zcorpan]

I suppose that works. classList would have an empty array? Is the array sorted lexically? Is it [SameObject]?

There could be concern that exposing an array opens up for misuse that is not possible with supports(), e.g. assuming that the first item is a particular token.

[phistuck]

Damn. If code becomes dependable on the order... That would be really, really sad. :(
Can we just assume that developers are not that hardcodedy? :(

This is a good incentive to change it every few months with a new great and shiney token. ;)

[zcorpan]

This is the Web... :-(

[yoavweiss]

I don't think we should go with an array, since there's really no reason to. It's harder & more expensive to code to (have to iterate over it to find if an item is there), and would implicitly encourage dependence on implementation details such as order.

[phistuck]

supports() might do the same, internally, anyway. But I do not really mind either way. supports() and hasSupportedTokens, or only supportedTokens[].

[domenic]

Please stop insisting on hasSupportedTokens; we have explained to you repeatedly in this thread why it is unnecessary.

[phistuck]

Please, @domenic, stop making it look like a fact when it is not. Opinions diverge on the matter.

[foolip]

Is plan A now to introduce DOMSupportedTokenList, or to have just DOMTokenList + "once and for all, deciding on which DOMTokenLists receive this upgrade"?

[domenic]

Plans A-C are about basically deciding what classList.supports("asdf") returns. In (A) it throws a TypeError since the supports method does not exist. In B it returns true. In C it returns false.

[foolip]

OK. If it returns true when there are no supported tokens, then there will be an ugly but practical way to see if an object as has any supported tokens: supportsTokens = !list.supports('neversupportedtoken').

(I don't think we should worry at all about the case where an attributes goes from having supported tokens to not having them.)

[yoavweiss]

We could also go with

D) throw if supports("whatever") is called on a DOMTokenList with no supported tokens. Since authors are not supposed to call classList.supports("something"), I think that can work well to remind them of that fact.

[phistuck]

This is an option. The only ugly thing about it is that try/catch patterns are generally less optimized in scripting engines, but I am not sure this should be considered here, as you can check it once and cache the result or something.

[zcorpan]

D works for me. InvalidAccessError?

[domenic]

TypeError

[yoavweiss]

Closing as this PR is not the right way to go. Will PR what we discussed here shortly.