dojo.attr() doesn't return current value of a text input field

[roman2]

Assuming inputNode is a node of type text input, dojo.attr(inputNode, "value") doesn't return current value of the field (i.e. what is entered into the field). However, inputNode.value does.

I've tested this on Firefox 3.0.7.

[bill]

I tried ​test_Menu.html and then in the console did dojo.attr("input1", "value").

That works correctly, BUT if I type a different value than "top-left" into the input, the dojo.attr() still returns the original value.

This isn't a new problem though, it happened in 1.2 also.

[James Burke]

This belongs in the "should attr be a general property and attribute getter/setter". There is a good case for it, but traditionally, with the exception of a limited set of properties it has not been. The current property whitelist approach will need to change.

Assigning to Eugene for now, but just as a placeholder (no pressure Eugene :)

[roman2]

Until this is changed, I'd ask that the documentation be modified to reflect the specialness of some attributes. Currently (​http://api.dojotoolkit.org/jsdoc/dojo/HEAD/dojo.attr), it says this: "Handles normalized getting and setting of attributes on DOM Nodes." This leads me to believe that it's been designed to handle ALL attributes. The documentation change should probably apply to attr on NodeList? too.

Thanks

[Eugene Lazutkin]

We need a general solution for that. Any ideas besides whitelisting "direct" attributes?

[bill]

I'm in favor of actually making it work, not sure if that counts as an idea or is just stating the obvious.

I am worried about this because I searched our code and we are unfortunately calling dojo.attr(foo, "value", ...) in many places. I'm suspicious there are hidden bugs there.

dijit/form/CheckBox.js: dojo.attr(this.focusNode, 'value', this.value);
dijit/form/ComboBox.js: return dojo.attr(item, "value");
dojox/form/DropDownSelect.js: dojo.attr(this.valueNode, "value", this.attr("value"));
dojox/form/FileUploader.js: dojo.attr(f, "value", this.postData[nm]);
dojox/form/PasswordValidator.js: dojo.attr(this.focusNode, "value", v);
dojox/form/Rating.js: dojo.attr(evt.target, "value"); (twice)

Plus there might be hidden usages like dojo.attr(node, attrName, attrValue); inside DTL etc.... or usages where "dojo" has been abbreviated to "d" or some other variable name, although I tried to search for those too.

[James Burke]

I was doing something like the following for jquery compat -- it is still a bit hacky/pseudocode, can probably be cleaned up:

Get:

var prop = node[propName];
if((propName in node) && !dojo.isObject(propName) && propName != "href"){
	return prop;
}else{
	return [node.getAttribute stuff here] || prop;
}

Set:

//The setter case. Figure out if value is a function.
var prop = node[propName];
if((propName in node) && !dojo.isObject(prop) && name != "href"){
	node[propName] = value;
}else{
	[do node.setAttribute work here = value];
}

We might still need to do the name normalization stuff for htmlFor, class, etc. In there too.

[Eugene Lazutkin]

Related to #7355.

[Eugene Lazutkin]

(In [17931]) html: cleaning dojo.attr() according to James' suggestions (thx, James Burke!), removing the "whitelist" of attributes, adding a test for "value", updating tests to account for non-standard IE6/7 behavior, !strict, fixes #8991, fixes #7355.

[bill]

[17931] breaks dijit/tests/_base/manager.html and possibly a bunch of other dijit tests (there are lots of tests failing in dijit now but I'm not sure when all the failures started; I only traced down this first error)

[bill]

hasAttr() for "disabled" and "tabindex" are both broken, see #9430.

[Eugene Lazutkin]

(In [18126]) Renaming "tabindex" to "tabIndex", !strict, refs #8991.

[Eugene Lazutkin]

(In [18127]) Renaming "tabindex" to "tabIndex", !strict, refs #8991.

[Adam Peller]

Please provide more explicit docs for getEffectiveAttrValue

[Eugene Lazutkin]

[18125] was not processed by the robot.

[Eugene Lazutkin]

Brain dump

Every DOM node has a list of attributes (attribute nodes), which correspond to attributes in the HTML markup, and a list of properties, which are node-specific values available directly on the node.

Attributes are manipulated by getAttribute(), setAttribute(), and removeAttribute(). If attribute is missing, null is returned by getAttribute().

Alternative interface includes attributes collection, and getAttributeNode(). The latter returns an object with three properties:

1. name --- read-only attribute name.
2. specified --- read-only Boolean flag.
3. value --- read/write attribute's value.

So any attribute can be missing, or present. In the latter case it has a value, which is a string of some sort. If an attribute was not specified but it has a default value per spec, it will be present with a value, but not specified.

Properties are always present, if they are allowed for a certain type of node. Their value type is defined by the DOM spec.

When a node is rendered, browsers takes into account both properties and attributes. It is possible to have a property and an attribute of the same name and for the same logical entity. According to the DOM spec properties are final authorities on the effective value of such entity.

Complications

It all sounds nice but different browsers exhibit different behavior:

• Some properties shadow attributes, and vice versa. It is quite possible that an attribute is missing, yet the corresponding property is readable returning a default value. And an assignment to such property can create the corresponding attribute too.
• Some corresponding properties and attributes may have totally different values. Example: disabled the property operates with Boolean values, while disabled the attribute uses strings with values "disabled" for true and "" for false.
• It is possible to have an attribute node present, yet its specified property is false. Example: for <input type="text"> IE produces an attribute with specified === false, and this value can be changed only once.
• It is possible to have both property and attribute out of sync with different values. The worst offender is IE.
• Removing an attribute node in same cases leaves the corresponding property value unchanged --- needs more investigation.
• Some default values are different on different browsers. Example: non-existent tabIndex is 0 on IE, and -1 on other browsers (depending on a node type). According to the spec it cannot be negative.
• Some properties and attributes are in use, yet there is no spec for them. Example: innerHTML.

Additional problem arises from the fact that browsers can be lax with enforcing upper/lower case for attribute names. Some browsers tolerate any case, some browsers require a certain camelCase as per spec. Example: tabIndex on IE.

dojo.hasAttr()

The current version of dojo.hasAttr() checks for the presence of an attribute node. If it is not present, or its property specified is false, the function returns a falsy value, otherwise it returns a truthy value.

The negative result does not mean that there is no effective value. It does not mean that there is no property with the same name. And on IE it does not mean that the attribute in question was not specified in the HTML markup.

dojo.attr()

The current version of dojo.attr() in the "set" mode does following actions:

1. Special processing for style.
2. Special processing for innerHTML.
3. If the value is a function, setting of an event handler is assumed, and specially processed.
4. If the value is Boolean, it forces setting a property, rather than an attribute.
5. If the name is in the special list, it forces a property setting.
6. Otherwise setAttribute() is used.

In the "get" mode it does following actions:

1. If the name is in the special list, and the existing value of a property is not undefined, the property value is returned.
2. If a value of the property is Boolean or a function, it is immediately returned.
3. If dojo.hasAttr() is positive, {{{getAttribute()}}'s value is returned.
4. Otherwise null is returned.

To be continued...

[Becky Gibson]

(In [18297]) fixes #9430 due to changes made to fix #8991, can no longer use hasAttr(node, "disabled") to test for the disabled attr set on an input field. Better to use dojo.attr(node,"disabled") as that will return the correct boolean value. !strict

[Becky Gibson]

I didn't realize that a commit comment with both "fixes" and a ticket number anywhere in the text would close all referenced tickets - I thought they had to be directly after the fixes keyboard. Thus, #8991 is NOT fixed by [18297] (above). Thus, reopening.

[Becky Gibson]

(In [18298]) refs #8991 - added tests for disabled attr on input element since referencing disabled via property or via getAttribute("disabled") returns different results on different browsers. This test would catch any changes to dojo.attr() to not reference disabled as node.disabled.

[bill]

Replying to becky:

I didn't realize that a commit comment with both "fixes" and a ticket number anywhere in the text would close all referenced tickets - I thought they had to be directly after the fixes keyboard. Thus, #8991 is NOT fixed by [18297] (above). Thus, reopening.

Well, I just reopened this ticket because the changes here ([17931]) broke dijit. If dijit is working now then this ticket can probably be closed again, although I'll leave it to Eugene.

[Eugene Lazutkin]

Replying to bill:

Well, I just reopened this ticket because the changes here ([17931]) broke dijit. If dijit is working now then this ticket can probably be closed again, although I'll leave it to Eugene.

I need to know if something is still broken, but I am not done with the ticket yet --- at the very least I need to document new addition as per peller's comment.

[Becky Gibson]

(In [18382]) refs #8991 updated disabled test to use valid markup (disabled="true" is NOT valid, disabled="disabled" is valid

[bill]

For an input with no type specified, this changes the return code from "text" to null. In dojo/tests/_base/html.html, try

dojo.attr("input-no-type", "type")

I guess that was intended, given that there's now a unit test checking the return code is null, although the behavior change is breaking dijit.Dialog (and could break other folks depending on the old behavior). I'll check in a fix to dijit.Dialog.

[Eugene Lazutkin]

Replying to bill:

For an input with no type specified, this changes the return code from "text" to null.

What browser?

[Eugene Lazutkin]

Replying to bill:

I guess that was intended, given that there's now a unit test checking the return code is null, although the behavior change is breaking dijit.Dialog (and could break other folks depending on the old behavior). I'll check in a fix to dijit.Dialog.

You can try dojo.getEffectiveAttrValue() --- it is supposed to return the effective value, and should be "text" in this case.

Hint: dojo.getEffectiveAttrValue() works like read-only dojo.attr(), but defaults to node properties rather than attributes. According to the spec it is supposed to be the authoritative source.

[bill]

Replying to elazutkin:

Replying to bill:

For an input with no type specified, this changes the return code from "text" to null.

What browser?

Presumably it happens in all browsers, assuming the unit test passes in all browsers, but I just tested on FF3.5 / mac.

[bill]

(In [18794]) Due to [18125], dojo.attr(input, "type") returns null rather than "text" for an <input> w/no type explicitly specified. Use dojo.getEffectiveAttrValue() instead.

Refs #8991 !strict.

[Foam Head]

Apologies if I'm out of order, but I just hit what appears to be this bug. While parsing some custom XML, I noticed all calls to dojo.attr(xmlDomNode, 'type') returned undefined/null. I boiled this down to a simple HTML example:

var mydiv = dojo.create('div', { type: 'mytype' });
console.log(dojo.attr(mydiv, 'type'));
console.log(mydiv.getAttribute('type'));

The first log prints undefined and the second properly shows "mytype". Other attribute names work fine, but "type" seems to be a special case that is somehow borked. This is with Dojo 1.3.2 running in FireFox? 3.5.1 on Windows XP SP3.

PS. I see this ticket has a 1.4 milestone. Any chance of seeing a fix in a 1.3 point release?

[Eugene Lazutkin]

Replying to Foam Head:

The first log prints undefined and the second properly shows "mytype".

Amazing. This is the code from dojo.attr() that reads attributes in three consecutive sections:

// getter
// should we access this attribute via a property or
// via getAttribute()?
value = node[propName];
if(forceProp && typeof value != "undefined"){
  // node's property
  return value; // Anything
}

The previous if never returns undefined by definition.

if(propName != "href" && (typeof value == "boolean" || d.isFunction(value))){
  // node's property
  return value; // Anything
}

The previous if can return only a Boolean and a function value.

// node's attribute
// we need _hasAttr() here to guard against IE returning a default value
return _hasAttr(node, attrName) ? node.getAttribute(attrName) : null; // Anything

The final line can return either node.getAttribute(attrName) (which is "mytype" according to your example) or null.

Yet undefined is returned somehow.

Are you sure you used the trunk for your test?

If it returns null it means that _hasAttr() fails somehow. It is very simple and hard to get wrong:

var _hasAttr = function(node, name){
  var attr = node.getAttributeNode && node.getAttributeNode(name);
  return attr && attr.specified; // Boolean
};

The alternative explanation would be that type has a special (magic) meaning in DOM, and should not be used on DOM nodes.

Still I'll check the test snippet to see what is going on. In any case it is intriguing.

[Foam Head]

As I mentioned in my first comment, I hit this bug in Dojo 1.3.2. If I knew how to test off of trunk (without needing to do a full source tree download and build), I'd be glad to do it.

For reference, if you load up 1.3.2, you can do the following in the FireBug? console to demonstrate the problem:

>>> dojo.version
1.3.2 (18832) major=1 minor=3 patch=2 revision=18832
>>> var foo = dojo.create('div', { name: 'myname', type: 'mytype' })
>>> foo
<div name="myname" type="mytype">
>>> dojo.hasAttr(foo, 'name')
true
>>> dojo.attr(foo, 'name')
"myname"
>>> foo.getAttribute('name')
"myname"
>>> dojo.hasAttr(foo, 'type')
true
>>> dojo.attr(foo, 'type')
>>> typeof dojo.attr(foo, 'type')
"undefined"
>>> foo.getAttribute('type')
"mytype"

Obviously I'd like to ensure this is fixed in trunk for 1.4, but I also think it warrants a bug fix in 1.3 point release. If you need me to open additional tickets or whatnot, just lemme know.

[Eugene Lazutkin]

You can go to the nightlies (​http://archive.dojotoolkit.org/) and run an test you like to load the base, e.g., ​http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/parser.html --- now you can do the research from the Firebug console.

I tried your scenario and it works properly in the trunk --- it looks like the last rewrite fixed this bug as well.

[Foam Head]

I was able to use a nightly build test to confirm dojo.attr(something_with_a_type_attr, 'type') does work. So for 1.4, I agree this problem is closed.

However, this still fails in Dojo 1.3.2. I don't know what your screening process for point release fixes is, but can you please bring this up for discussion? IMHO it warrants a fix in a point release, but it may not warrant the creation of a point release; i.e. if a 1.3.3 release is going to happen for other reasons, this fix should definitely be included. If you need me to create a ticket for this, please let me know.

[Eugene Lazutkin]

It is hard to tell about including this code in the upcoming point release (if it happens).

Typically we include only simple bugfixes. The change I did was a complete rewrite --- I felt the code was buggy, too complex, and in some places illogical, because some underlying reasons for it were not clear. It means that it will break some code for sure --- I know that because it broke some code in Dijit/DojoX, which needed some light patching.

While it is relatively simple to replace dojo.attr(), it would be a major chore to extract related changes from other parts of the toolkit.

I frequently run my projects from a snapshot of Dojo trunk --- we try to keep it stable and in many cases it works fine. If this option is acceptable to you (obviously it requires some trials), you can try to use the trunk. Otherwise, you can patch your own copy of Dojo, or package it as a separate custom function, which will be eliminated when the next release is out.

[Foam Head]

I agree that you rarely want to back prop a redesign into a point release, but it shouldn't stop a new fix in a point release. Yes, this is throw away work, but I've since confirmed this is a bigger issue and may warrant the effort.

Looking at the code for dojo.attr, a dojo.attr(domNode, 'attrName') invocation basically just runs this code:

// getter
// should we access this attribute via a property or
// via getAttribute()?
var prop = _attrProps[name.toLowerCase()];
if(prop){
	return node[prop];
}
var attrValue = node[name];
return (typeof attrValue == 'boolean' || typeof attrValue == 'function') ? attrValue
	: (d.hasAttr(node, name) ? node.getAttribute(name) : null);

The first few lines with prop looked a little odd to me, so I checked out _attrProps:

// non-deprecated HTML4 attributes with default values
// http://www.w3.org/TR/html401/index/attributes.html
// FF and Safari will return the default values if you
// access the attributes via a property but not
// via getAttribute()
var _attrProps = {
	colspan: "colSpan",
	enctype: "enctype",
	frameborder: "frameborder",
	method: "method",
	rowspan: "rowSpan",
	scrolling: "scrolling",
	shape: "shape",
	span: "span",
	type: "type",
	valuetype: "valueType",
	// the following attributes don't have the default but should be treated like properties
	classname: "className",
	innerhtml: "innerHTML"
}

So the intent is to get the attribute's value from the DOM node when the attribute was not specified by markup and has a default value. This makes perfect sense and works on nodes that have default attribute values. The problem is it doesn't work on nodes that don't have those default values. That's why you get this (from the FireBug? console with Dojo 1.3.2):

>>> dojo.attr(dojo.create('div', {}), 'type')
>>> dojo.attr(dojo.create('div', { type: 'mytype' }), 'type')
>>> dojo.attr(dojo.create('input', {}), 'type')
"text"
>>> dojo.attr(dojo.create('input', { type: 'reset'}), 'type')
"reset"

So the code handles its intended case of providing defaults, but it accidentally short-circuits the code for nodes without the default values. There are a couple of ways to solve this, but one solution might look something like this:

// getter
// should we access this attribute via a property or
// via getAttribute()?
var attrValue = node[name];
if (typeof attrValue == 'boolean' || typeof attrValue == 'function') {
	return attrValue;
} else if (d.hasAttr(node, name)) {
	return node.getAttribute(name);
} else {
	var prop = _attrProps[name.toLowerCase()];
	return (prop ? node[prop] : null);
}

This is untested and you may need to swap the first and second cases, but you get the idea. It's not a huge fix and it should be easy to ensure all of the _attrProps attributes work in all node types.

If there's anything else I can do to help with this, please let me know.

[Eugene Lazutkin]

Thanks for the proposed patch. It is small enough, so I don't think we need a CLA from you. If we are to make 1.3.3, I'll try to put it in.

[Eugene Lazutkin]

Adding to the Brain Dump above (elazutkin):

The Purpose

Why???

dojo.attr

It was explained to me that dojo.attr() is meant to accomplish two things:

1. Some stuff is an attribute, and some stuff is a property. Confusingly in many cases you can read/write both, which can work in some browsers and bomb in others. dojo.attr() was supposed to hide this duality in most common code.
2. Another obvious reason was to eliminate browser differences in getting/setting attributes. For example, some browsers expect certain attribute names in a correct case.

dojo.removeAttr

I think it was added mostly for completeness. What it does it resolves a node, using dojo.byId(), and can correct a name for some known attributes --- basically it is a one-liner.

dojo.hasAttr

Amazingly this little function complicates the whole picture.

Obviously properties are always there, so it deals with attributes. It is used to see if certain attributes were set on a node (in HTML). It can do its role to a point, because there are some problems most notably with IE.

Somehow it is expected that results of dojo.hasAttr() and dojo.attr() should match. It leads to dojo.attr() returning null when dojo.hasAttr() returns false, otherwise it should return some sane attribute value. But the absence of an attribute doesn't mean that there is no effective (e.g., default) value for it. It can be found in a corresponding property. The current dojo.attr() does not support reading it without an attribute present, and we cannot change its behavior without breaking a lot of code.

That's why I added dojo.getEffectiveAttrValue(), which works only for reading values, and consults a property first falling back to attributes, when there is no property available. It returns the default/effective value, if it is available.

Exceptions

Some properties do not shadow attributes, e.g., className, innerHTML, and so on. They should be always accessed as node properties. To enforce that there is a small list of such names.

Some attributes have non-matching property names for various reasons (e.g., programming language conflicts). They have their own table.

While majority of properties are lower-case names, some of them use the camel case. There is a special table for that too.

There is a special processing for style by deferring to dojo.style(), and for innerHTML to handle read-only cases (HTML in IE).

[Eugene Lazutkin]

In order to close this ticket I need to revisit the documentation and document dojo.getEffectiveAttrValue(). BTW I hate the name, it is too long. Name ideas?

[dante]

I agree, getEffectiveAttValue is a horrible name for a public base API.

re: docs; please add this to release notes, and expand inline docs to contain at least one example (and wordage to explain stuff from this ticket, about why this is to be used over .attr)

[Eugene Lazutkin]

(In [20873]) Renaming getffectiveAttrValue to getNodeProp as agreed on the meeting, !strict, refs #8991.

[Eugene Lazutkin]

(In [20874]) Renaming getEffectiveAttrValue to getNodeProp as agreed on the meeting, !strict, refs #8991.

[Eugene Lazutkin]

(In [20875]) Renaming getEffectiveAttrValue to getNodeProp as agreed on the meeting, !strict, refs #8991.

[Eugene Lazutkin]

Need to document dojo.getNodeProp() and review the existing documentation of dojo.attr().

[Eugene Lazutkin]

Reworked documentation for:

• dojo.attr()
• dojo.style()

Added documentation for:

• dojo.getNodeProp()

Documented undocumented functions (why weren't they documented???):

• dojo.hasAttr()
• dojo.removeAttr()
• dojo.NodeList.attr*()

I've noticed that dojo.NodeList is largely undocumented.

[bill]

For reference, jquery went through the same growing pains, see ​http://stackoverflow.com/questions/5874652/prop-vs-attr.

I'm still unclear of when application code should set properties vs. setting attributes. For example, I wonder how dijit should be setting tabIndex and aria roles/states of nodes.