`propertychange` on an input[type=radio] with this.checked fires standard onChange #2170

Closed
DimitarChristoff opened this Issue Dec 13, 2011 · 9 comments

Comments

Projects
None yet
6 participants
@DimitarChristoff
Member

DimitarChristoff commented Dec 13, 2011

http://jsfiddle.net/5CvBb/5/

document.getElements("input").addEvents({
    "change": function() {
        alert("change");
    }
});


document.id("dom").removeClass("validation-failed");

run this with a simple dom of:
<input type="radio" name="foo" value="yes" class="validate-one-required" /> yes <input id="dom" type="radio" name="foo" value="no" checked="checked" /> no
possibly to do with the new Event Delegation when w/o eventListenerSupport, this does not break in 1.3.2 or earlier.

@arian

This comment has been minimized.

Show comment
Hide comment
@arian

arian Dec 13, 2011

Member

I think it has to do with 4522d61

ping @csuwldcat

Member

arian commented Dec 13, 2011

I think it has to do with 4522d61

ping @csuwldcat

@DimitarChristoff

This comment has been minimized.

Show comment
Hide comment
@DimitarChristoff

DimitarChristoff Dec 13, 2011

Member

oh, very cool.

/*<ltIE9>*/
delete Element.NativeEvents.propertychange;
delete Element.Events.change;
/*</ltIE9>*/

does not fix my issue, in fact, it makes intended change events really unpredictable. is the bug to do with the fact that radios and checkboxes don't really fire a change but a propertychange instead? hence, the bug is in the condition which fires a change on any propertychange event, as long as the this.checked is true.

Member

DimitarChristoff commented Dec 13, 2011

oh, very cool.

/*<ltIE9>*/
delete Element.NativeEvents.propertychange;
delete Element.Events.change;
/*</ltIE9>*/

does not fix my issue, in fact, it makes intended change events really unpredictable. is the bug to do with the fact that radios and checkboxes don't really fire a change but a propertychange instead? hence, the bug is in the condition which fires a change on any propertychange event, as long as the this.checked is true.

@DimitarChristoff

This comment has been minimized.

Show comment
Hide comment
@DimitarChristoff

DimitarChristoff Dec 13, 2011

Member

yep. anything you d that does a propertychange fires the change event. http://jsfiddle.net/5CvBb/7/ with a .set("data-attrib") does it too.

Member

DimitarChristoff commented Dec 13, 2011

yep. anything you d that does a propertychange fires the change event. http://jsfiddle.net/5CvBb/7/ with a .set("data-attrib") does it too.

@DimitarChristoff

This comment has been minimized.

Show comment
Hide comment
@DimitarChristoff

DimitarChristoff Dec 13, 2011

Member

erm. fixed though I am not sure how 'safe' it is.

 return !!((this.type != 'radio' || this.checked) && event.event.propertyName == 'checked');
Member

DimitarChristoff commented Dec 13, 2011

erm. fixed though I am not sure how 'safe' it is.

 return !!((this.type != 'radio' || this.checked) && event.event.propertyName == 'checked');
@csuwildcat

This comment has been minimized.

Show comment
Hide comment
@csuwildcat

csuwildcat Dec 13, 2011

Contributor

Wow, I can't believe I didn't check to make sure the attribute that changed was "checked". Haven't tested the patch, but I will tomorrow. Thanks for catching this.

Contributor

csuwildcat commented Dec 13, 2011

Wow, I can't believe I didn't check to make sure the attribute that changed was "checked". Haven't tested the patch, but I will tomorrow. Thanks for catching this.

@ibolmo

This comment has been minimized.

Show comment
Hide comment
@ibolmo

ibolmo Dec 13, 2011

Member

I'll try to include this in 1.4.3, otherwise will be included in 1.4.4 (two week sprint) or 1.5 (depends on feature set)

Member

ibolmo commented Dec 13, 2011

I'll try to include this in 1.4.3, otherwise will be included in 1.4.4 (two week sprint) or 1.5 (depends on feature set)

@DimitarChristoff

This comment has been minimized.

Show comment
Hide comment
@DimitarChristoff

DimitarChristoff Dec 13, 2011

Member

@ibolmo please consider DimitarChristoff/mootools-core@6abff54 instead as it was the only safe-ish option that did not break normal input[type=text] change, read #2172 (comment)

Member

DimitarChristoff commented Dec 13, 2011

@ibolmo please consider DimitarChristoff/mootools-core@6abff54 instead as it was the only safe-ish option that did not break normal input[type=text] change, read #2172 (comment)

@fanxiaolong

This comment has been minimized.

Show comment
Hide comment
@fanxiaolong

fanxiaolong Jun 12, 2014

你们会中文吗?

你们会中文吗?

@tom-field

This comment has been minimized.

Show comment
Hide comment

我会

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment