Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOMSettableTokenList needs an associated content attribute #81

Closed
cdumez opened this issue Sep 21, 2015 · 19 comments
Closed

DOMSettableTokenList needs an associated content attribute #81

cdumez opened this issue Sep 21, 2015 · 19 comments

Comments

@cdumez
Copy link

cdumez commented Sep 21, 2015

https://dom.spec.whatwg.org/#interface-domsettabletokenlist

A DOMSettableTokenList object is equivalent to a DOMTokenList object without an associated attribute.

The HTML specification is using the DOMSettableTokenList in places where there is an associated attribute. For e.g. HTMLAnchorElement.ping [1]:
[PutForwards=value] readonly attribute DOMSettableTokenList ping;

It needs to be a DOMSettableTokenList because PutForwards needs to be able to set the 'value' attribute on the DOMSettableTokenList.

[1] https://html.spec.whatwg.org/multipage/semantics.html#the-a-element

@cdumez
Copy link
Author

cdumez commented Sep 22, 2015

Also see HTMLOutputElement.htmlFor [2]:
[PutForwards=value] readonly attribute DOMSettableTokenList htmlFor;

When setting htmlFor, you would definitely expect the 'for' attribute to be updated as well. However, the DOM specification for DOMSettableTokenList.value [3] setter does not say anything about it. It only says to update the tokens, it does not say we should run the update steps [4].

I have verified in Firefox that setting HTMLOutputElement.htmlFor to a given string does update the associated 'for' attribute.

[2] https://html.spec.whatwg.org/#htmloutputelement
[3] https://dom.spec.whatwg.org/#dom-domsettabletokenlist-value
[4] https://dom.spec.whatwg.org/#concept-DTL-update

@ArkadiuszMichalski
Copy link
Contributor

Have you any test case? This behavior often depends on the particular browser. Some time ago I test DOMSettableTokenList for HTMLLinkElement.sizes but noticed different behaviour in Firefox, Chrome and IE11:

<!DOCTYPE html>
<html>

<head>
    <link sizes="32x32">
    <script>
        window.onload = function(){

            var box = document.getElementById("box");
            var link = document.getElementsByTagName("link")[0];
            var sizes_attr = link.attributes[0];

            box.innerHTML += "link.sizes: " + link.sizes // "32x32" - read set
                + "<br> sizes_attr.value: " + sizes_attr.value; // "32x32" - read attr

            link.sizes = "32x32 48x48 48x48"; // change set (not change attr)

            box.innerHTML += "<br><br>link.sizes: " + link.sizes // "32x32 48x48" - read set
                + "<br> sizes_attr.value: " + sizes_attr.value; // "32x32" - read attr

            sizes_attr.value = "48x48"; // change attr (change set)?

            box.innerHTML += "<br><br>link.sizes: " + link.sizes // "48x48" - read set
                + "<br> sizes_attr.value: " + sizes_attr.value; // "48x48" - read attr

        }
    </script>

</head>
<body>
    <div id="box">
</body>
</html>

Chrome:
link.sizes: 32x32
sizes_attr.value: 32x32

link.sizes: 32x32 48x48 48x48
sizes_attr.value: 32x32

link.sizes: 48x48
sizes_attr.value: 48x48

Firefox:
link.sizes: 32x32
sizes_attr.value: 32x32

link.sizes: 32x32 48x48 48x48
sizes_attr.value: 32x32 48x48 48x48

link.sizes: 48x48
sizes_attr.value: 48x48

IE11:
link.sizes: undefined
sizes_attr.value: 32x32

link.sizes: 32x32 48x48 48x48
sizes_attr.value: 32x32

link.sizes: 32x32 48x48 48x48
sizes_attr.value: 48x48

I'm not sure what behaviour is correct, changing attr's value via attr.value should change sets (from DOMSettableTokenList) or not? If yes where exactly do I find this definition? In DOM I see just this:
"This specification uses and applicable specifications can use the hooks an attribute is set, an attribute is changed, an attribute is added, and an attribute is removed, for further processing of the attribute’s value."
For class we have that info in DOM:
"When an element is created that has a class attribute or when an element’s class attribute is set, set the element’s classes to the new value, parsed."
But for other content attributes I don't see any additional ifnos in HTML.

@cdumez
Copy link
Author

cdumez commented Oct 17, 2015

@ArkadiuszMichalski: You are talking about the opposite case (i.e. Should updating the content attribute also update the associated DOM(Settable)TokenList token?). For the record, I would also like for this case to be clarified but this is not what this bug is about.

This bug is about updating the associated content attribute when the DOMSettableTokenList tokens are updated. The DOM spec is clear about the fact that it should happen for DOMTokenList but says that DOMSettableTokenList does not have an associated content attribute. I think this difference in behavior is odd, especially considering DOMSettableTokenList is used in the HTML spec for attributes that reflect content attributes.

@ArkadiuszMichalski
Copy link
Contributor

@cdumez: I presented both case in this example for DOMSettableTokenList:

changing set of tokens > affect content attribute (Firefox, not Chrome and not IE^)
changing content attribute > affect set of tokens (Firefox and Chrome, not IE*)
^ - IE doesn't operate on DOMSettableTokenList here (just DOMString)

and wondering if the same happen for other cases (not only HTMLLinkElement.sizes). And I see that DOM is precise here (even for DOMTokenList and class, where we have in prose what should be done when changing attr value, the opposite situation is described in sets algo) but HTML is very economical and to general for both of these interface :
"If a reflecting IDL attribute has the type DOMTokenList or DOMSettableTokenList, then on getting it must return a DOMTokenList or DOMSettableTokenList object (as appropriate) whose associated element is the element in question and whose associated attribute's local name is the name of the attribute in question. The same DOMTokenList or DOMSettableTokenList object must be returned every time for each attribute."

Of course it would be useful to know why DOMSettableTokenList should not affect the content attribute and vice versa (when we are modifying one of them), compatibility reason?

@annevk
Copy link
Member

annevk commented Oct 19, 2015

Looking at the history of this API in HTML I think DOM is simply wrong here. Not sure how that happened. DOMSettableTokenList should have an associated content attribute.

As for the other question, setting value should probably just set the content attribute which in turn runs the set parser.

Paging @bzbarsky and @domenic to make sure I'm not missing anything.

@annevk annevk changed the title "A DOMSettableTokenList object is equivalent to ..." DOMSettableTokenList needs an associated content attribute Oct 19, 2015
@ArkadiuszMichalski
Copy link
Contributor

Ah, if you pain attention to clean up sets, then this bug should be somehow fixed too:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=20660
because now its hard to determine from the HTML spec what should be done here.

@bzbarsky
Copy link

Yeah, I think DOM is just wrong here. And yes, setting value should set the content attr. That's what Firefox implements, yes?

@ArkadiuszMichalski
Copy link
Contributor

@bzbarsky: yes, but not in Chrome (question is why, maybe spec bug). DOMSettableTokenList should not just work as DOMTokenList + one additional argument (value)?

@bzbarsky
Copy link

@ArkadiuszMichalski I'm not sure what you're asking. The whole point of DOMSettableTokenList when it was introduced was to be just like DOMTokenList but settable, in the sense that it was taking attributes that used to be string-typed and that people had set like this: foo.ping = something and aiming to preserve that behavior. This was done by giving it a settable attribute named value and making every single consumer do [PutForwards=value].

As far as Chrome's implementation goes, it does the following for places where the HTML spec uses DOMSettableTokenList:

  1. HTMLElement.dropzone -- not implemented
  2. HTMLLinkElement.sizes -- as your behavior above: updates the internal state of the DOMSettableTokenList but not the actual attribute, which is broken.
  3. HTMLAnchorElement.ping -- implemented correctly per sanity: setting a.ping or a.ping.value changes the value of a.getAttribute("ping").
  4. HTMIFrameElement.sandbox -- implemented correctly per sanity.
  5. HTMLAreaElement.ping -- implemented correctly per sanity.
  6. HTMLTableCellElement.headers -- implemented as a string, not a DOMSettableTokenList.
  7. HTMLOutputElement.htmlFor -- implemented brokenly like HTMLLinkElement.sizes.

So my take on it is that Chrome has correct implementations of this for various places (e.g. sandbox/ping), has two broken ones (HTMLOutputElement.htmlForandHTMLLinkElement.sizes`), and the spec is buggy.

@ArkadiuszMichalski
Copy link
Contributor

@bzbarsky: Some times ago I tested only HTMLLinkElement.sizes in Chrome (because Firefox not support this yet) and see that setting new value don't actual content attribute (what per spec was correct because DOMSettableTokenList has not associated any content attribute). Now your explanation is sufficiently detailed and I do not have any more questions, just waiting for fixing.

annevk added a commit that referenced this issue Oct 20, 2015
@annevk
Copy link
Member

annevk commented Oct 20, 2015

TODO:

@ArkadiuszMichalski
Copy link
Contributor

@bzbarsky: have another question, you wrote "HTMLTableCellElement.headers -- implemented as a string, not a DOMSettableTokenList" << but this string type is correct or not? I ask because test ping variant in Chrome and Firefox (HTMLAnchorElement, HTMLAnchorElement) and they both return string (not DOMSettableTokenList object), but you wrote that "So my take on it is that Chrome has correct implementations of this for various places (e.g. sandbox/ping)". Per IDL [PutForwards=value] should not only affect setting, not getting?

And Anne today make changes for DOMSettableTokenList:
"Setting the value attribute must set an attribute value for the associated element using associated attribute’s local name and the given value."
So this must only change asociated content attribute (without touching internal set, removing duplicate or unnecessary white space from setting string)?

@bzbarsky
Copy link

but this string type is correct or not?

Define "correct"? The HTML spec says HTMLTableCellElement.headers should be a DOMSettableTokenList but no one actually implements it that way. Whether that's "no one implements it that way yet" or "no one implements it that way and the spec should just change", I can't tell you.

I ask because test ping variant in Chrome and Firefox (HTMLAnchorElement, HTMLAnchorElement) and they both return string

I assume you meant (HTMLAnchorElement, HTMLAreaElement)?

In Chrome dev (48.something), HTMLAnchorElement.ping is a DOMSettableTokenList as far as I can tell. In Chrome release (46.something) and Chrome beta (47.something), it's a string. In Firefox it's a string. My testing in #81 (comment) was in Chrome dev.

So this must only change asociated content attribute (without touching internal set, removing
duplicate or unnecessary white space from setting string)?

It should act exactly the way setAttribute acts, I would think.

@bzbarsky
Copy link

Or perhaps setAttributeNS with the relevant namespace and localname.

@ArkadiuszMichalski
Copy link
Contributor

Define "correct"?

I mean correct per actual specs (HTML/DOM/Web IDL and other) what I suppose all browsers should go (or try) if that DOMSettableTokenList and [PutForwards=value] appear on this documents and no one protested.

Ohh, I check only stable Chrome 46 so this is the reason for my surprise, now downolading canary and devs and testing what is happening here.

In Chrome 48 dev: HTMLAnchorElement.ping, HTMLAreaElement.ping, HTMIFrameElement.sandbox sets is 'synchronized' with content attribute (in both way), changing one of them is reflected in a second.

<script type = "text/javascript">

    var el_name = "a";
    var el_attr = "ping";
    var el = document.createElement(el_name);

    console.log(el[el_attr].constructor); // DOMSettableTokenList() { [native code] }
    console.log(el[el_attr].length); // 0 - set is empty
    console.log(el.getAttribute(el_attr)); // null - att not exist

    el[el_attr].value = "1 2 3";

    console.log(el[el_attr].length); // 3
    console.log(el[el_attr].toString()); // 1 2 3
    console.log(el[el_attr].contains(3)); // true
    console.log(el.getAttribute(el_attr)); // 1 2 3 

    el[el_attr].add(4);
    console.log(el[el_attr].toString()); // 1 2 3 4
    console.log(el.getAttribute(el_attr)); // 1 2 3 4

    el.setAttribute(el_attr, 0);
    console.log(el.getAttribute(el_attr)); // 0
    console.log(el[el_attr].toString()); // 0
    console.log(el[el_attr].value); // 0

</script>

@annevk
Copy link
Member

annevk commented Nov 11, 2015

@cdumez is "Christophe Dumez" in the acknowledgments still your preferred spelling? Or is that somebody else?

@annevk annevk closed this as completed in ff81234 Nov 11, 2015
@cdumez
Copy link
Author

cdumez commented Nov 11, 2015

@annevk Thanks for asking. I am "Christophe Dumez" (and already in the acknowledgments) but I go by "Chris Dumez" these days. If you could update it, it would be great, thanks.

annevk added a commit that referenced this issue Nov 12, 2015
@annevk
Copy link
Member

annevk commented Nov 12, 2015

You got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants