Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#11899 closed bug (wontfix)

Problems with .attr() and [attribute] selector in IE7, after event DOM manipulation.

Reported by: samuel.h.r.nunes@… Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 1.7.2
Keywords: Cc:
Blocked by: Blocking:

Description

Manipulation of events using the DOM Event Model creates problems for .attr() and the [attribute] selector in IE7.

After you manipulate e.g. the "element.onclick" directly with the DOM:

  • jQuery(element).attr('onclick') will return "undefined",
  • jQuery('[onclick]') will not find the element anymore.

Other browsers (FF, Chrome, IE8, IE9) work as expected. See here for a detailed test case: http://jsfiddle.net/LXkhk/5/

Change History (7)

comment:1 Changed 7 years ago by Rick Waldron

Resolution: wontfix
Status: newclosed

Sorry, but this is outside of jQuery's scope

http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers

comment:2 Changed 7 years ago by samuel.h.r.nunes@…

Thanks for the fast response.

Well, inline event handlers may be outside of jQuery's scope, but let me present my arguments into why I think that this particular issue is an exception. After all the text does say "In nearly all cases, ..." ;)

  • This problem is recent. In jsFiddle it only shows up with jQuery 1.7.1 or higher. Here's a working example using jQuery 1.6.4: http://jsfiddle.net/LXkhk/6/,
  • It has the implication that the [onclick] attribute selector becomes unreliable when doing DOM event manipulation,
  • There is nothing wrong with using the DOM Event Model to manipulate events, is there?

comment:3 in reply to:  1 ; Changed 7 years ago by samuel.h.r.nunes@…

Replying to rwaldron:

Sorry, but this is outside of jQuery's scope

http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers

Bumping this, since I forgot to click "reply" when I commented back yesterday.

You seem to have closed this ticket very quickly, probably without even looking at the example (just 2 minutes after it was created). But I think that this deserves more consideration, and that the "out of scope" description doesn't apply:

  • The main point here is that this is a regression in jQuery 1.7. See my previous comment for the jsFiddle link that proves it.
  • The issue itself is not about getting inline event handlers working, but that .attr() doesn't work as specified in the API. The expected behaviour would be to retrieve the initial (string) value of the attribute, since the DOM manipulation should affect the property only. That's what happens on all tested browsers except IE7.
  • Additionally the [attribute] selector also fails in these conditions, which is probably related.
  • At the very least, a note detailing that .attr() and [attribute] fail under these special conditions would be expected in the documentation. Otherwise, there's no way to see this as not a bug.

Please don't just ignore this feedback. It's highly unmotivating to contribute detailed bug reports with jsFiddle examples, if they're closed 2 minutes after creation without any chance of reconsideration.

comment:4 in reply to:  3 Changed 7 years ago by mikesherov

Replying to samuel.h.r.nunes@…:

Replying to rwaldron:

Sorry, but this is outside of jQuery's scope

http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers

Bumping this, since I forgot to click "reply" when I commented back yesterday.

You seem to have closed this ticket very quickly, probably without even looking at the example (just 2 minutes after it was created). But I think that this deserves more consideration, and that the "out of scope" description doesn't apply:

  • The main point here is that this is a regression in jQuery 1.7. See my previous comment for the jsFiddle link that proves it.

A regression is when supported functionality stops working. If you were using unsupported use cases for an API that were just happening to be working and now they are not, that is not a regression.

  • The issue itself is not about getting inline event handlers working, but that .attr() doesn't work as specified in the API. The expected behaviour would be to retrieve the initial (string) value of the attribute, since the DOM manipulation should affect the property only. That's what happens on all tested browsers except IE7.

http://api.jquery.com/attr/ states "Get the value of an attribute for the first element in the set of matched elements." It also says "Cross-browser consistency: The .attr() method reduces such inconsistencies." Notice it doesn't say "eliminates", but rather "reduces". If you're using unsupported functionality, we make no gaurantees.

  • Additionally the [attribute] selector also fails in these conditions, which is probably related.
  • At the very least, a note detailing that .attr() and [attribute] fail under these special conditions would be expected in the documentation. Otherwise, there's no way to see this as not a bug.

We note in http://api.jquery.com/attr/ "Note: Attribute values are strings with the exception of a few attributes such as value and tabindex." Perhaps we should link to the http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers here as well.

Please don't just ignore this feedback. It's highly unmotivating to contribute detailed bug reports with jsFiddle examples, if they're closed 2 minutes after creation without any chance of reconsideration.

Your feedback was not ignored. Rather, it was seen, and the appropriate response given. You're entitled to come here and plead your case all you want, but it was not ignored. We have a list of things we don't fix, specifically because there are better ways to do what you're trying to do, and because it wastes time for us trying to fix it.

You should not be unmotivated simply because we responded to your issue by closing it. We welcome any and all bug reports. We just close them quickly if they are invalid or a duplicate. Please don't mistake our curtness for rudeness. We got lots of bug reports per day, and we don't like invalid tickets piling up.

comment:5 Changed 7 years ago by Rick Waldron

Thanks Mike.

@samuel everything above is true ;)

ps. I totally looked at the fiddle. I even spent time afterward trying to figure out what you were hoping to accomplish with this approach :P

comment:6 Changed 7 years ago by dmethvin

Sorry it seemed that we were too abrupt, but really once something is in the wontfix category it's not like we're doing regression tests to see how much we won't fix them. We test for the things where we are concerned about regressions and this isn't one of them.

When you're using inline event handlers you're outside jQuery's event system and we can't guarantee many of the things that we do for events attached by jQuery. IE7 has the "attroperty" problem where it confused attributes and properties. We fix that for the cases that we need to keep consistent, and it's quite a lot of work. Since inline event handlers are outside the envelope we're not looking for or fixing any changed behavior there.

I know it can be frustrating to find a problem that has caused issues for your code, spend some time documenting it, and then get this response. It doesn't change the fact that inline attributes have not been considered best practice in web development for years, and as a practical matter we can't support them as first-class event citizens within jQuery.

comment:7 Changed 7 years ago by samuel.h.r.nunes@…

Thanks for the more in-depth replies :)

@mikesherov and @dmethvin: Your explanations are clear, and I accept the reasonings behind them. Unfortunately, for a small team maintaining a large application, it's impossible to move away from inline event handlers any time soon. So we're stuck with having to find workarounds after updating to the latest jQuery :(

To be clear, I never expected inline event handlers to be "fixed" as you do for events attached by jQuery. I'm aware of their limitations and that they are considered "bad practice" nowadays.

However, I really thought that it makes sense to keep fixing the IE7 "attroperty" problem for event attributes as long as IE7 is supported (especially since you did that up to 1.6.4). But of course only you can know the bad implications that this had, and why you decided to drop it from your code.

@rwaldron: Sorry if my frustration sounded as bitter before. To be honest, I don't have a specific thing that I'm trying to accomplish here. Instead, I have a bunch of regressions for things that were working before. A good example is the "[onclick]" selector which is used widely in our code. It seems that I will have to implement a custom filter to replace it.

Note: See TracTickets for help on using tickets.