Intent to implement and ship: DOMTokenList based feature detection for <link> and <iframe>

[Yoav Weiss]

Contact email

yo...@yoav.ws / ywe...@akamai.com

Spec

Relevant spec changes:

https://github.com/whatwg/dom/pull/103

https://github.com/whatwg/dom/pull/107

https://github.com/whatwg/html/pull/340

Summary

We are lacking a way to feature detect features where an attribute can receive a set of predefined keywords. In such features, there's no way to detect support for a certain keyword, as keywords are added over time.

Recent changes to DOMTokenList enable to detect keyword support in a backward compatible way by adding them using the `add()` method, and observing the return value.

Motivation

Lack of feature detection capabilities leads to UA string detection, anarchy and mayhem. This change would enable us to incrementally add keyword based syntax while having a reliable feature detection mechanism for supported keywords.

Compatibility Risk

Low.

The feature detection itself is backwards compatible, since currently `add()` returns undefined, which is falsy. Also, `relList` itself is already shipped in Firefox, so there's little risk that introducing it would cause compat issues.

Ongoing technical constraints

None.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

OWP launch tracking bug

crbug.com/557597

Link to entry on the feature dashboard

https://www.chromestatus.com/feature/4663631736209408

Requesting approval to ship?

Yes.

If preferable, I can postpone exposing `relList` until <link rel=preload> (which is the main use-case for feature detection on <link rel>) is shipped.

[Mike West]

-mike

Non-owner's LGTM. I'd like to see this implemented for `iframe@sandbox`, as it suffers from the same fate as `link@rel`. In particular, implementing this will allow Google's anti-malvertising team to feature-detect support for `allow-popups-to-escape-sandbox` rather than whitelisting particular user agents, potentially increasing the number of ads they can lock inside a sandbox.

Do you plan on tackling the `sandbox` implementation as well as `rel`?

-mike

[PhistucK]

I strongly support this proposal. So ridiculously simple. :)

Hm, looks like you are only implementing it for <link> and <iframe>, but the linked issue discusses the general case. Can you file an issue for those two specific cases instead and update the entry?

I think every implemented instance of DOMTokenList should follow this -

https://code.google.com/p/chromium/codesearch#search/&q=DOMTokenList%20file:idl&sq=package:chromium&type=cs

https://code.google.com/p/chromium/codesearch#search/&q=DOMSettableTokenList%20file:idl&sq=package:chromium&type=cs

Since, seemingly, there are not a lot of instances, perhaps support this in all of them (and then keep that issue :))?

☆PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Harald Alvestrand]

I must be dense. So we're suggesting a token list that allows a range of values to be set, and only these values, and claim that trying to set a value and inspecting whether or not we can set it is a simple mechanism for detecting whether it is allowed or not (apparently setting the value has no semantic effect - or at least none that's visible in the PRs I read).

Why doesn't the attribute simply list the acceptable values?

[Yoav Weiss]

On Thu, Nov 19, 2015 at 10:18 AM, Mike West <mk...@chromium.org> wrote:

Non-owner's LGTM. I'd like to see this implemented for `iframe@sandbox`, as it suffers from the same fate as `link@rel`. In particular, implementing this will allow Google's anti-malvertising team to feature-detect support for `allow-popups-to-escape-sandbox` rather than whitelisting particular user agents, potentially increasing the number of ads they can lock inside a sandbox.

Do you plan on tackling the `sandbox` implementation as well as `rel`?

Thanks! I intend to implement it for sandbox as well, since once everything required for `rel` is added, `sandbox` impl is trivial.

[Yoav Weiss]

On Thu, Nov 19, 2015 at 10:52 AM, Harald Alvestrand <h...@google.com> wrote:

I must be dense. So we're suggesting a token list that allows a range of values to be set, and only these values, and claim that trying to set a value and inspecting whether or not we can set it is a simple mechanism for detecting whether it is allowed or not (apparently setting the value has no semantic effect - or at least none that's visible in the PRs I read).

Why doesn't the attribute simply list the acceptable values?

Traditionally, "supported lists" have had a tendency to get out-of-sync. Tying feature detection to part of the feature's functionality would help to avoid false negatives upon feature implementation, and false positives in case of feature removal.   

[Yoav Weiss]

On Thu, Nov 19, 2015 at 10:34 AM, PhistucK <phis...@gmail.com> wrote:

I strongly support this proposal. So ridiculously simple. :)

Hm, looks like you are only implementing it for <link> and <iframe>, but the linked issue discusses the general case. Can you file an issue for those two specific cases instead and update the entry?

I think every implemented instance of DOMTokenList should follow this -

https://code.google.com/p/chromium/codesearch#search/&q=DOMTokenList%20file:idl&sq=package:chromium&type=cs

https://code.google.com/p/chromium/codesearch#search/&q=DOMSettableTokenList%20file:idl&sq=package:chromium&type=cs

Since, seemingly, there are not a lot of instances, perhaps support this in all of them (and then keep that issue :))?

For classList, this isn't really relevant, since we can't have a concept of "supported tokens" (since any value is supported). 

The only other implemented instance where it may be relevant is HTMLOutputElement's htmlFor, which admittedly I'm not that familiar with.

All in all, I plan to implement generic support into the DOMTokenList and DOMSettableTokenList classes, and specific implementation can then inherit from them and add supported tokens where it makes sense.

[PhistucK]

So will classList.add("foo") return true?

☆PhistucK

[Yoav Weiss]

On Thu, Nov 19, 2015 at 12:38 PM, PhistucK <phis...@gmail.com> wrote:

So will classList.add("foo") return true?

Yeah, since "validation steps" returns true if the attribute has no "supported tokens" defined. 

[Ilya Grigorik]

On Thu, Nov 19, 2015 at 1:18 AM, Mike West <mk...@chromium.org> wrote:

Non-owner's LGTM. I'd like to see this implemented for `iframe@sandbox`, as it suffers from the same fate as `link@rel`. In particular, implementing this will allow Google's anti-malvertising team to feature-detect support for `allow-popups-to-escape-sandbox` rather than whitelisting particular user agents, potentially increasing the number of ads they can lock inside a sandbox.

Mike mentioned this above, but for a bit of context: feature detecting rel={preconnect, prefetch, prerender} support is a frequently requested feature from many developers, and this solve that problem. In other words, another non-owner LGTM! 

[Chris Harrelson]

I'm a little confused about the proposed scope of this Intent. Is it to return a boolean from all DOMTokenList objects? Or just some of them?

--

[Yoav Weiss]

On Thu, Nov 19, 2015 at 10:34 PM, Chris Harrelson <chri...@chromium.org> wrote:

I'm a little confused about the proposed scope of this Intent. Is it to return a boolean from all DOMTokenList objects? Or just some of them?

 

The intent is to return a boolean for the `add()` method of all DOMTokenList objects. That boolean would be true for all these objects where validation is not relevant (e.g. classList) or not yet implemented. Once that's in place, specific objects (e.g. HTMLLinkElement's relList and HTMLIFrameElement's sandbox) can add their own validation logic, with their specific supported keywords.

[Christian Biesinger]

Hmm but then how can websites rely on this? If we don't support validation for a certain property  we will return true just like for a value that we know we support. How can website tell the difference?

-Christian

[Rick Byers]

Can you show an example of what code you expect developers to write here to support browsers with and without this API (maybe this needs an explainer or blog post or something)?  In particular, you'd want clients to feature-detect this API by treating 'false' and 'undefined' differently, right?

Rick

[smaug]

On 11/20/2015 12:52 AM, Yoav Weiss wrote:

>
> On Thu, Nov 19, 2015 at 10:34 PM, Chris Harrelson <chri...@chromium.org <mailto:chri...@chromium.org>> wrote:
>
> I'm a little confused about the proposed scope of this Intent. Is it to return a boolean from all DOMTokenList objects? Or just some of
>

> The intent is to return a boolean for the `add()` method of all DOMTokenList objects. That boolean would be true for all these objects where
> validation is not relevant (e.g. classList) or not yet implemented. Once that's in place, specific objects (e.g. HTMLLinkElement's relList and
> HTMLIFrameElement's sandbox) can add their own validation logic, with their specific supported keywords.

So this possibly breaks feature usage on browsers which support, say rel=prefetch, but don't have DOMTokenList API updated.
!!tokenList.add("foo") is false when add() returns void, but true when add() returns bool as per the change.
Doesn't sound too good.



-Olli

>
> --
> You received this message because you are subscribed to the Google Groups "blink-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org

> <mailto:blink-dev+...@chromium.org>.

[Yoav Weiss]

On Fri, Nov 20, 2015 at 3:41 AM, Rick Byers <rby...@chromium.org> wrote:

Can you show an example of what code you expect developers to write here to support browsers with and without this API (maybe this needs an explainer or blog post or something)?  In particular, you'd want clients to feature-detect this API by treating 'false' and 'undefined' differently, right?

Rick

Example code for such detection could be something like:

var isLinkRelSupported = function(rel) {

    var link = document.createElement("link");

    if (!link.relList || link.relList.add("whatever")) {

        // RelList or its feature detection are not yet implemented

        // Keep doing whatever it is that you've been doing until now

        return "maybe";

    }

        

    if (link.relList.add(rel)) {

        // We *know* the rel value is supported, since add() returned true for it, but falsy elsewhere

        return "supported";

    } 

    // We *know* the rel value is not supported. Polyfill or fallback.

    return "not supported";    

} 

We plan to add a note to the preload/resource-hints specs explaining that API. A blog post would probably also be a good idea :)

[Yoav Weiss]

On Fri, Nov 20, 2015 at 4:11 AM, smaug <sm...@welho.com> wrote:

On 11/20/2015 12:52 AM, Yoav Weiss wrote:

On Thu, Nov 19, 2015 at 10:34 PM, Chris Harrelson <chri...@chromium.org <mailto:chri...@chromium.org>> wrote:

    I'm a little confused about the proposed scope of this Intent. Is it to return a boolean from all DOMTokenList objects? Or just some of

The intent is to return a boolean for the `add()` method of all DOMTokenList objects. That boolean would be true for all these objects where
validation is not relevant (e.g. classList) or not yet implemented. Once that's in place, specific objects (e.g. HTMLLinkElement's relList and
HTMLIFrameElement's sandbox) can add their own validation logic, with their specific supported keywords.

So this possibly breaks feature usage on browsers which support, say rel=prefetch, but don't have DOMTokenList API updated.
!!tokenList.add("foo") is false when add() returns void, but true when add() returns bool as per the change.
Doesn't sound too good.

While there is a some risk of breakage here, I doubt its significance.

Searching for relList on GitHub brings back a single project with very few stars, and even that project does not examine the return value of `add()`.

[Philip Jägenstedt]

I agree that there's a potential problem here, if any DOMTokenList attribute ever get upgraded to having supported tokens where it currently does not. I've filed a spec issue and PR:

https://github.com/whatwg/dom/issues/111

https://github.com/whatwg/dom/pull/112

LGTM1 to implement and ship with that change, but if you have even the slightest concern with that, let's discuss more!

Philip

[Boris Zbarsky]

On 11/20/15 12:28 AM, Yoav Weiss wrote:
> Example code for such detection could be something like:

I really hope people don't actually write code like that, because it
totally doesn't do what you claim it does.

> var isLinkRelSupported = function(rel) {
> var link = document.createElement("link");
> if (!link.relList || link.relList.add("whatever")) {
> // RelList or its feature detection are not yet implemented
> // Keep doing whatever it is that you've been doing until now
> return "maybe";

If the feature detection is not yet implemented, then
link.relList.add("whatever") will return undefined, which is falsy, so
this early return won't be taken.

> // We *know* the rel value is not supported.

No, you don't. All you know is that it's not supported _or_ the new API
is not implemented yet.

A correct feature test would have to go something like this:

var isLinkRelSupported = function(rel) {
var link = document.createElement("link");

// Must use a rel value we KNOW UAs support here.
if (!link.relList || !link.relList.add("stylesheet")) {

// RelList or its feature detection are not yet implemented
// Keep doing whatever it is that you've been doing until now
return "maybe";
}

if (link.relList.add(rel)) {
// We *know* the rel value is supported, since add() returned

// true for it.

return "supported";
}
// We *know* the rel value is not supported. Polyfill or fallback.
return "not supported";
}

> We plan to add a note to the preload/resource-hints specs explaining
> that API. A blog post would probably also be a good idea :)

The fact that even people on this list can't figure out how to use the
API correctly is not a good sign for people getting it right in the
wild. I have to admit I was much more optimistic about this API until I
saw that. :(

-Boris

[Boris Zbarsky]

On 11/20/15 9:04 AM, Boris Zbarsky wrote:
> A correct feature test would have to go something like this:

Or even simpler, something like this:

var isLinkRelSupported = function(rel) {
var link = document.createElement("link");

if (!link.relList) {
// RelList not yet implemented

// Keep doing whatever it is that you've been doing until now
return "maybe";
}

var result = link.relList.add(rel);

if (result) {

// We *know* the rel value is supported, since add() returned
// true for it.
return "supported";
}

if (result === undefined) {
// New API not supported yet.
return "maybe";

}

// We *know* the rel value is not supported. Polyfill or fallback.
return "not supported";
}

But at that point, what do we really win from using a boolean return
value? Would we be better off doing what I just suggested in
https://github.com/whatwg/dom/issues/111#issuecomment-158411312 ?

-Boris

[Yoav Weiss]

I'm fine with that. Spec PR at https://github.com/whatwg/dom/pull/114 

[Yoav Weiss]

On Sat, Nov 21, 2015 at 5:35 AM, Yoav Weiss <yo...@yoav.ws> wrote:

I'm fine with that. Spec PR at https://github.com/whatwg/dom/pull/114 

After discussions on previous PR, a different solution is PRed at https://github.com/whatwg/dom/pull/123

[Chris Harrelson]

Hi Yoav,

Are you requesting approval to ship this new approach?

Chris

[Yoav Weiss]

On Thu, Dec 3, 2015 at 6:27 PM, Chris Harrelson <chri...@chromium.org> wrote:

Hi Yoav,

Are you requesting approval to ship this new approach?

Yes, assuming it will get merged into the spec.

Just to clarify that new approach, using it to perform feature detection may look something like:

var tokenListSupports = function(tokenList, token) {

  if (!tokenList || !tokenList.supports) {

    return false;

  }

  try {

    return tokenList.supports(token);

  } catch (e) {

    if (e instanceof TypeError) {

      console.log("The DOMTokenList doesn't have a supported tokens list");

    } else {

      console.log("That shouldn't have happened");

    }

  }

};

var linkSupportsPreload = tokenListSupports(document.createElement("link").relList, "preload"); // true

var classListSupportsWhatever = tokenListSupports(document.createElement("div").classList, "whatever"); // undefined + console.log

var linkSupportsWhatever = tokenListSupports(document.createElement("link").relList, "whatever"); // false

[Philip Jägenstedt]

Will you include the merge of DOMSettableTokenList into DOMTokenList in this work? It's not strictly related, but the change happened only because of this discussion, and it would be nice to find out if merging them will create a mess in Blink.

In either case, LGTM1 to implement and ship the new DOMTokenList.prototype.supports approach once it's in the spec.

[Yoav Weiss]

On Thu, Dec 3, 2015 at 9:11 PM, Philip Jägenstedt <phi...@opera.com> wrote:

Will you include the merge of DOMSettableTokenList into DOMTokenList in this work? It's not strictly related, but the change happened only because of this discussion, and it would be nice to find out if merging them will create a mess in Blink.

I could work on that as well, but I consider that an orthogonal change, which may require its own I2S.

What mess do you think it might create? Internal implementation issues or compat issues with existing content?

[Philip Jägenstedt]

Internal implementation issues was my concern, mentioned in https://github.com/whatwg/dom/pull/120#issuecomment-159874863

Other than content extending DOMSettableTokenList.prototype (why not?) I don't think there's must compat risk.

It is an orthogonal issue, though, so a separate I2S sounds like a good idea if you want to try tackling it.

Philip

[TAMURA, Kent]

DOMTokenList.prototype.supports LGTM2

--

TAMURA Kent
Software Engineer, Google

[Simon Pieters]

On Thu, 03 Dec 2015 23:58:25 +0100, Philip Jägenstedt <phi...@opera.com>
wrote:

> Other than content extending DOMSettableTokenList.prototype (why not?) I
> don't think there's must compat risk.

FWIW I researched that in httparchive before making the PR and didn't find
anything.

https://github.com/whatwg/dom/issues/119#issuecomment-159856813

I also looked for DOMTokenList.prototype but didn't find anything
problematic there either.

> <zcorpan> [...] there is a possibility of course that pages set
> prototype.value on DOMTokenList and assume it does not affect
> DOMSettableTokenList. unlikely though
> <zcorpan> all i can find is classList polyfills

http://krijnhoetmer.nl/irc-logs/whatwg/20151126#l-563

--
Simon Pieters
Opera Software

[Philip Jägenstedt]

Thanks, Simon! It seemed like a safe change without research, but actually checking is even better :)

[Chris Harrelson]

LGTM3

--