When setting a boolean from the IDL attribute, the content attribute value should be the attribute name

RESOLVED INVALID

Status

()

defect
RESOLVED INVALID
9 years ago
9 years ago

People

(Reporter: mounir, Assigned: Ms2ger)

Tracking

({html5})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

9 years ago
In other words:
When i set: element.foo = true;
I should have: element.getAttribute('foo') == 'foo';

I will provide a patch as soon as possible (later today). This is really easy to fix.
Assignee

Comment 1

9 years ago
Posted patch Patch (obsolete) — Splinter Review
Sorry for stealing your bug; I still had this patch lying around.
Assignee: nobody → Ms2ger
Attachment #436729 - Flags: review?(bzbarsky)
Reporter

Comment 2

9 years ago
No problem, that's fine :)
Comment on attachment 436729 [details] [diff] [review]
Patch

nsDependentAtomString, please.
Attachment #436729 - Flags: review?(bzbarsky) → review-
Unless nsAttrValue will share a stringbuffer with the passed-in string?
Assignee

Comment 5

9 years ago
Posted patch Patch v2Splinter Review
Attachment #436729 - Attachment is obsolete: true
Attachment #436741 - Flags: review?(bzbarsky)
Comment on attachment 436741 [details] [diff] [review]
Patch v2

r=me, but sicking should look too.
Attachment #436741 - Flags: superreview?(jonas)
Attachment #436741 - Flags: review?(bzbarsky)
Attachment #436741 - Flags: review+
Ugh, why?! Does this match what IE does?
Reporter

Comment 8

9 years ago
I don't know what IE does (I don't have a Windows system around to test) but that's what is in the specs in HTML5 specs at the moment and I do not think that is going to break compatibility as, AFAIK, a boolean attribute is considered true as long as it is present. To set it to false, it has to be removed.

Webkit is doing it (at least, I checked for autofocus attribute) and Opera was doing it and it looks like it is now setting attribute to 'true'. I suppose this is a regression.

Why don't you like that change ?
Well, for one it means that we'll serialize to a very ugly syntax. Currently a disabled input element gets serialized to

<input disabled>

With this patch we'd serialize to

<input disabled="disabled">

Which is something you'd rarely see someone write by hand. 

I looked at what IE does, and unfortunately its .getAttribute implementation is a bit weird. In this case it returns "true". But If you serialize, using for example .innerHTML, you'll get the same short serialization as gecko.


The other concern I have here is simply wasting memory and cycles. I really can't see a reason to stick the full string "disabled" into memory.


I think we should argue for a spec change here given that in basically all cases people set boolean attributes to the empty string when using markup. So it'd be nice to be consistent when setting those same boolean attributes through the DOM.

And given that browsers do different things here, compatibility doesn't seem to be a big argument one way or another.
Comment on attachment 436741 [details] [diff] [review]
Patch v2

Minusing for now. Let me know if you'd like me to raise this with WhatWG or the HTML WG.
Attachment #436741 - Flags: superreview?(jonas) → superreview-
I changed the spec.
Assignee

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → INVALID
No documentation needed, I suppose.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.