Skip to main content

Bug Tracker

Side navigation

#9102 closed bug (wontfix)

Opened May 04, 2011 04:04PM UTC

Closed May 05, 2011 01:45PM UTC

Last modified March 14, 2012 05:18AM UTC

attr() doesn't always return a string in jQuery 1.6 and lower

Reported by: Andy E Owned by: Andy E
Priority: low Milestone:
Component: attributes Version: 1.6
Keywords: neededdocs Cc:
Blocked by: Blocking:

For Internet Explorer 7 and older, attr() may return a boolean or function type for certain attributes, for example "checked" or "onclick" respectively.

This is due to the old getAttribute bug that returns the DOM property value instead of the attribute string, which was finally fixed in IE 8. As a workaround, jQuery should make use of getAttributeNode rather than getAttribute.

This solution can be demonstrated by the following fiddle: Run it in IE's compatibility mode or IE 7 or IE 6.

Attachments (0)
Change History (10)

Changed May 04, 2011 05:01PM UTC by rwaldron comment:1

component: unfiledattributes
owner: → timmywil
status: newassigned

Changed May 04, 2011 05:22PM UTC by timmywil comment:2

priority: undecidedlow

For onclick, I'm not sure we want to add the code to handle those cases. If we do, we'll want to handle all of the inline event handlers as special cases. hideFocus is not supported in any browser except IE, so I don't think we want to worry about that one either or we'll have to add it to propFix. Regardless, type is already a string and checked will consistently be a string in 1.6.1, although I'm not sure how that one is returning a boolean in 1.6. Anyway, avoiding the use of getAttributeNode wherever possible is a good idea.

Changed May 04, 2011 05:28PM UTC by timmywil comment:3

OK, IE returns a boolean with getAttribute when there's no value. This is moot tho with the boolean attributes changes for 1.6.1.

Changed May 04, 2011 05:35PM UTC by timmywil comment:4

Changed May 04, 2011 05:45PM UTC by Andy E comment:5


Why is avoiding the use of getAttributeNode wherever possible a good idea?

I used hidefocus purely as an example of a boolean attribute, but there are many others (checked, selected, contenteditable, disabled, etc...). Casting those to strings, however, would fix the problem just as well.

I filed this bug because I came across a real-world scenario where jQuery was causing problems with attr and the oncontextmenu attribute. Would it be a problem to check the 0-2 substring of the requested attribute for "on" and use getAttributeNode in that case?

Changed May 04, 2011 06:59PM UTC by dmethvin comment:6

owner: timmywilAndy E
status: assignedpending

What is the practical problem in leaving this the way it is? I'd like to see some pseudo-realistic code that needs this to work cross-browser. As the code stands now it's possible to see if an onXXXX handler is attached for example, especially now that 1.6 supports .prop().

Changed May 04, 2011 07:24PM UTC by timmywil comment:7

The other boolean attributes you mention will return strings in 1.6.1 except for contenteditable, since it's property is actually contentEditable (we might want to add that one to propFix). I see hidefocus as a different matter tho.

As for your question, getAttributeNode should only be used when getAttribute is not sufficient because it is a drop in performance every time it is used. It is debatable whether it is worth the extra code to support unifying types for on* attributes since interacting with those is generally not a good idea anyway. It's unnecessary to edit the attribute when you should be editing the property.

Changed May 05, 2011 09:06AM UTC by Andy E comment:8

status: pendingnew

Fix it or don't, it doesn't really matter to me. I reported it because someone else ran into the problem and I thought one of the ideas behind jQuery was cross-browser interoperability where possible.

I find it interesting you discuss performance, because attr as a getter doesn't really have much to offer beyond getAttribute - especially if it's not offering a consistent result across all the supported browsers - maybe jQuery should advise that users just use getAttribute instead? It's also one thing to tell me that interacting with those attributes isn't a good idea, but I already know that. jQuery is used by many, many users that don't have the slightest idea of the correct approach to anything (just look for all the users using keyup to detect text input), and it's those people that you need to convey that message to.

For what it's worth, I don't think there would be much code involved and you could keep a minimal performance hit by using feature detection on init, followed by only using getAttributeNode in the browsers that don't return the correct type:

    var getAttributeNodeRequired,
        tmp = document.createElement("div");

    tmp.innerHTML = "<span onclick=void(0)></span>";
    getAttributeNodeRequired = typeof tmp.firstChild.getAttribute("onclick") === "function";

    /* ... */

    $.fn.attr = function (name /*, etc, ... */) {
        /* ... */
        if (getAttributeNodeRequired && name.slice(0,2) == "on")
            return this[0].getAttributeNode(name);
        /* ... */

If it's not to be fixed, then it might be a good idea to warn users in the docs for attr instead.

Changed May 05, 2011 01:45PM UTC by dmethvin comment:9

keywords: → needsdocs
resolution: → wontfix
status: newclosed

Andy, I think your suggestion for users to use getAttribute for these cases is the best course of action. It just isn't common enough to justify the extra work to fix for a case (inline handlers) that we recommend against anyway.

I've marked the ticket as needing a docs change ("Attributes are generally strings, but in IE6/7 attr may return boolean for checked or a function for onclick.")

Note that people often WANT .attr() to return truthy/falsy for boolean attributes, we tried to change that in 1.6 and had to back it out for 1.6.1. So It's unlikely anyone is complaining about that.

Changed June 28, 2011 01:20PM UTC by addyosmani comment:10

keywords: needsdocsneededdocs

Docs updated as per dmethvin's note: Should we need to expand on this please feel free to re-add the needsdocs tag and I'll update accordingly.