Posted by ericduran on February 21, 2012 at 7:12pm
25 followers
| Project: | jQuery Update |
| Version: | 7.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | reviewed & tested by the community |
Issue Summary
This issue is coming from #1386294: Release jQuery 1.7 for Drupal 7.
The problem with the states.js code is documented over at http://drupal.org/node/1386294#comment-5518008
Changed .attr to .prop
/misc/states.js252: return this.prop('checked');
389: $(e.target).prop('checked', e.value);
Comments
#1
Stick states.js in jquery_update/replace/misc/1.7/states.js.
#2
Works perfectly for me. Thanks!
#3
Oops, no it doesn't. It breaks on the
valuestate, when referencing aselectelement.#4
Sorry, I'm having a theme error. Works fine.
#5
Thanks for the review! http://drupalcode.org/project/jquery_update.git/commit/07db916
#6
Automatically closed -- issue fixed for 2 weeks with no activity.
#7
Like in #1448494: jQuery 1.7: field_ui.js use attr for property I think is better if we change that upstream. So I keep it open so we don't forget to remove what we did.
EDIT: I didn't open a new issue to keep stuff in context.
#8
I was having problems with http://drupal.org/project/field-conditional-state not working properly (read: at all) with jquery_update 2.x-dev using jquery 1.7. I tracked this down to a slight difference in states.js that happened after Rob Loach's commit above. Specifically, this commit coming out of #735528: FAPI #states: Fix conditionals to allow OR and XOR constructions changes the Drupal.states.Dependent.prototype.compare function argument list, and consequently breaking field_conditional_states extension of that function.
This is an unfortunate side-effect of replacing core files which may change, so I agree this should ultimately be dealt with upstream, but I'm not really sure how to go about that. For the interim, in case others run across similar issues, I've attached a patch which replaces jquery_update's copy of states.js with the most current version from core, and re-applies Rob Loach's fixes to make it work for 1.7.
#9
The title of the issue suggests that this is solved upstream on jquery's side. Is there an issue filed for that in their tracking system?
#10
Reading the code of states.js I think the problem is here:
states.Trigger.states = {
// 'empty' describes the state to be monitored
empty: {
// 'keyup' is the (native DOM) event that we watch for.
'keyup': function () {
// The function associated to that trigger returns the new value for the
// state.
return this.val() == '';
}
},
checked: {
'change': function () {
return this.attr('checked');
}
},
We should replace
return this.attr('checked');by something like than return true or false (I think now we can have undefined...)
I have to try it to be sure, but perhaps it can give some pointer to other.
Because now is broked, my advice will be to make a patch on state.js than works with core and jquery update, so we can after to send it upstream.
#11
Spiderman's patch really needs to go to next release.
#12
I had problems with the states.js that comes with the module as well. Same problem as here #1815896
After applying the patch in #8 the problems are solved. So i hope this patch moves into the next release.
However i did find a small bug. When there is a select multiple field, and there is a condition that should hide another field when this select multiple field is empty. Then the initial trigger will not do anything cause the .val() of the select multiple when empty is 'null'. And the function that updates the statuses checks initially that the new value is actually new. And since it compares with null, it will think that nothing changed and the condition will not apply.
So when the select multiple (dependee) is empty, it will not hide the dependent initially (on page load for example). as it should.
Attached a little tweak that should work. Just a workaround until there is a proper solution.
#13
#8 fixes it for me also
Tis the wrong issue for the patch (since this is about fixing it upstream and not one of the several 'states are broken' issues), but it's a solid patch and fixes a bad issue, so perhaps it can go in soon?