Bug Tracker

Ticket #6911 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

.live('click',handler) should not fire on disabled button in IE

Reported by: yoursunny Owned by: danheberden
Priority: high Milestone: 1.5.1
Component: event Version: 1.5
Keywords: Cc:
Blocking: Blocked by:

Description

Take the following snippet:

<div id="a"> <input type="button" disabled="disabled" id="b1" value="button1"> </div>

$(':button','#a').live('click',function(){ });

The live click event does not fire in Firefox 3.6, but fires in IE8 on Win7. Since the button is disabled, I think the correct implementation should not fire live click event on #b1.

Attachments

jquery-live-disabled.htm Download (810 bytes) - added by yoursunny 4 years ago.
reproduce code

Change History

Changed 4 years ago by yoursunny

reproduce code

comment:1 Changed 4 years ago by snover

  • Priority set to high
  • Status changed from new to open
  • Version changed from 1.4.2 to 1.4.3
  • Milestone changed from 1.4.3 to 1.next

comment:2 Changed 4 years ago by snover

  • Milestone changed from 1.4.4 to 1.4.5

Retargeting due to severity of regressions in 1.4.3.

comment:3 Changed 4 years ago by snover

  • Milestone changed from 1.4.5 to 1.4.4

Retargeting for 1.4.4 as per John’s request.

comment:4 Changed 4 years ago by snover

  • Owner set to danheberden
  • Status changed from open to assigned

comment:5 Changed 4 years ago by anonymous

Submitted pull request to disable events on disabled items:  http://github.com/jquery/jquery/pull/76

comment:6 Changed 4 years ago by danheberden

Heh, guess it helps to log in before doing that :p Pull req:  http://github.com/jquery/jquery/pull/76

comment:7 Changed 4 years ago by snover

  • Milestone changed from 1.4.4 to 1.4.5

Retargeting to next minor release.

comment:8 Changed 4 years ago by dmethvin

  • Status changed from assigned to closed
  • Resolution set to fixed

comment:9 Changed 4 years ago by danheberden

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:10 Changed 4 years ago by danheberden

That commit was backed out - I will provide a new one.

comment:12 Changed 4 years ago by snover

  • Status changed from reopened to assigned

comment:13 Changed 4 years ago by john

  • Status changed from assigned to closed
  • Resolution set to fixed

Landed.

comment:14 Changed 4 years ago by jitter

  • Milestone changed from 1.4.5 to 1.5

Move fixed tickets to appropriate milestone

comment:15 Changed 4 years ago by marksteward@…

Is this correct? It seems to me the fix should be to check currentTarget, not target.

Also, it's not limited to IE - it can be reproduced on Chrome 9.0.597.45. See  http://forum.jquery.com/topic/live-inconsistency-with-disabled-buttons.

comment:16 Changed 4 years ago by danheberden

comment:17 Changed 4 years ago by ms7821

danheberden: thanks. I think this might really be a separate bug, which is half fixed by your commit.

It's possible to receive events for a disabled <button> element as well as <input type="button">:

 http://jsfiddle.net/Fz2F3/ (jQuery 1.4.4)  http://jsfiddle.net/Fz2F3/1/ (jQuery 1.5)

IE7 fires click for all of them, passing the button as the target and currentTarget. Chrome fires only for the <button> with a child element, passing that as the target. However, currentTarget is set to the button, so changing target.disabled to currentTarget.disabled should extend the fix to include Chrome.

I didn't realise 1.5 had already been released, so I'll open a new ticket specifically for buttons on Chrome.

Mark

comment:18 Changed 4 years ago by danheberden

  • Status changed from closed to reopened
  • Version changed from 1.4.3 to 1.5
  • Resolution fixed deleted
  • Milestone changed from 1.5 to 1.5.1

Well in the end it's the same root issue, i'll go a head and re-open this one. Thanks for your work!

comment:19 Changed 4 years ago by ms7821

Ahh, do you want me to delete http://bugs.jquery.com/ticket/8165?

comment:20 Changed 4 years ago by danheberden

#8165 is a duplicate of this ticket.

comment:21 Changed 4 years ago by danheberden

Marked as dup - that'll link them together :)

comment:22 Changed 4 years ago by snover

  • Status changed from reopened to open

comment:23 Changed 4 years ago by danheberden

  • Status changed from open to closed
  • Resolution set to fixed

While at first glance "currentTarget" sounds like a reasonable fix, it isn't.

The firing event is still the element that is NOT disabled. So this fix would involve checking every click events parents for an element that's disabled. That isn't reasonable, as you can imagine.

So while we can easily check if event.target.disabled, checking the parent just in case isn't as easy. I mean easy as in not penalizing non-disabled actions with a crap load of work.

In this context, if you plan on attaching to elements that might be children of disabled elements then by all means, place that check in your own code.

Version 0, edited 4 years ago by danheberden (next)

comment:24 Changed 4 years ago by danheberden

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:25 Changed 4 years ago by danheberden

DaveMethvin pummeled me until i saw a way to do it - all better in  https://github.com/jquery/jquery/pull/237

comment:26 Changed 4 years ago by john

  • Status changed from reopened to closed
  • Resolution set to fixed

Landed.

comment:27 Changed 4 years ago by ms7821

Awesome to see, thanks danheberden!

Note: See TracTickets for help on using tickets.