In the same spirite than #1480568: use $.attr with false instead of empty string I would like to fix states.js to make it compatible with jQuery we have in core *and* more recent version.
I'm explaining the problem with new jquery version in #1498858: [meta] attr and prop.
The corresponding issue on jQuery Update #1448490: Remove the states.js overwrite as it was fixed upstream.
From my first reading of the code I think the problem is in
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');
}
},
checked.change can certainly return undefined. So we need to return false if we have anythings but true.
So somethings like that (untested)
checked: {
'change': function () {
if(this.attr('checked') === true){
return true;
}else {
return false;
}
}
},
Comment | File | Size | Author |
---|---|---|---|
#8 | states.patch | 294 bytes | droplet |
Comments
Comment #1
gagarine CreditAttribution: gagarine commentedComment #2
marcingy CreditAttribution: marcingy commentedDrupal 7 does not support versions of jquery higher than 1.4.
Comment #3
gagarine CreditAttribution: gagarine commentedThe script was working on 1.4 but it was already not the recommended way to use the API. So I think it will not hurt to fix it and as a side effect it will make it works on jQuery 1.6.1+ and save countless hours to a lot of peoples.
Comment #4
Renee S CreditAttribution: Renee S commentedGiven the number of installs of jQuery Update for people who want to used advanced jQuery, and given that this fix preserves backwards compatibility, why not?
Comment #5
marcingy CreditAttribution: marcingy commentedSo jquery update ships with replace file for states in 1.7+, so really the issue is one for jquery update not core.
Comment #6
nod_It's ok we can update core to make it easier on contrib, we just have to not break stuff.
The only way this moves along is if a patch is posted :)
Comment #7
nod_also, proper version.
Comment #8
droplet CreditAttribution: droplet commentedmaybe this way or access DOM directly. I think it will be accepted
Comment #9
goron CreditAttribution: goron commentedPatch in #8 worked for me
Comment #10
Renee S CreditAttribution: Renee S commentedComment #11
gagarine CreditAttribution: gagarine commentedPatch #8 look good. It's one of the recommended way to solve this problem http://api.jquery.com/attr/
Comment #12
nod_+1 on RTBC too
Comment #13
Renee S CreditAttribution: Renee S commented#8 is the way to go. Works beautifully.
Comment #14
hass CreditAttribution: hass commentedThat's really all? Just wondering why jquery_update has a states.js with tons of changes... Why are there so many differences?
Comment #15
oriol_e9gTested! This works, makes the live easier to developers and don't break anything so... why not? RTBC+
Comment #16
hass CreditAttribution: hass commentedJust to make sure this is not getting missed by the maintainer of D7.
Comment #17
gagarine CreditAttribution: gagarine commented@hass I never get why they "didn't want" to fix drupal core when is possible.... for a long time I think nobody really dig into this issue.
Comment #17.0
gagarine CreditAttribution: gagarine commentedtypo
Comment #18
presleyd CreditAttribution: presleyd commentedI've tested this with and without jquery_update now for a couple of days and I can't find any problems. It causes jquery_update to not need to keep their own copy of states.js and fixes conditional_fields for jquery 1.7.
Comment #19
David_Rothstein CreditAttribution: David_Rothstein commentedComment #20
David_Rothstein CreditAttribution: David_Rothstein commented7.24 was a security release (that's why I removed the tag). Seems like this should definitely go into the next bugfix release, but no reason for it to be a release blocker either.
Comment #21
sbrattla CreditAttribution: sbrattla commentedAny chance this will go into any upcoming bug fix release of Drupal?
Comment #22
oriol_e9gComment #23
oriol_e9gComment #24
sbrattla CreditAttribution: sbrattla commentedThis question is probably mostly due to my uncomplete knowledge of the process ahead, but what will make this patch go into a next release?
Is it the fact that it's tagged with a certain tag, or the fact that it is set as "Reviewed & Tested By The Community" - or does the patch have to be applied by a maintainer?
Comment #25
droplet CreditAttribution: droplet commented2 months. it seems the maintainer skipped this patch for some reason ? Dear maintainers, can you share your thought?
Comment #26
System Lord CreditAttribution: System Lord commentedI'm not afraid to say it...I'm confused as all heck! I need jquery_update to be 1.7. My CF 7.x-3.0-alpha1 doesn't like it. So, #8 patch is all I need?
I think I'm confused because on another similar thread there is a #8 patch that I "thought" fixed this problem, but the patch is completely different.
Would anyone mind telling me which patch I need to make CF play well with jquery_update 1.7?
Comment #27
Renee S CreditAttribution: Renee S commentedSystem Lord: Likely. You can replace states.js with the jQuery Update version, but this patch should fix most wayward checkbox behaviour, so you might not need it.
Comment #28
System Lord CreditAttribution: System Lord commentedWould /project/jqeries work for this issue?
Comment #29
System Lord CreditAttribution: System Lord commentedSorry, I'm still confused. Going back to my #26 do I need the #8 patch from this thread or the #8 patch from https://drupal.org/node/1448490, which is where gagarine brought this discussion from?
Comment #30
System Lord CreditAttribution: System Lord commentedAlso, should I be using the DEV at this point?
Comment #31
droplet CreditAttribution: droplet commented@System Lord,
Please report your problems with details in new thread. May be report it to "CF 7.x-3.0-alpha1" issue thread ?
Comment #32
System Lord CreditAttribution: System Lord commentedI'm currently using the alpha1, but it's giving me the same problems described in this thread (or at least where this thread originated from). I'm posting here because it sounds like patch #8 is a solution if I use the DEV. This is what I'm trying to find out. Let me phrase my question differently...
If I use the DEV and install patch #8 above it will solve the jquery issue?
Additionally, look at gagarines third paragraph at the top of this thread. In the thread he references there: #1448490: fix states.js upstream and remove the overwrite in jQuery Update, there seems to be another solution patch #8. Hence my confusion.
Comment #33
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis fix went into core:
https://drupal.org/node/735528
Comment #34
gagarine CreditAttribution: gagarine commented8: states.patch queued for re-testing.
Comment #35
gagarine CreditAttribution: gagarine commented@SocialNicheGuru I don't get it? Do you mean #735528: FAPI #states: Fix conditionals to allow OR and XOR constructions also fix this one?
Comment #36
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/f362e6a
Comment #37
System Lord CreditAttribution: System Lord commented+1 #35
Comment #38
SocialNicheGuru CreditAttribution: SocialNicheGuru commented#33, yes
Comment #39
gagarine CreditAttribution: gagarine commentedYes! Just before new year!
Comment #40
System Lord CreditAttribution: System Lord commentedOk, what's the bottom line here? lol What fixed what, where?
Comment #42
DrCord CreditAttribution: DrCord commentedThe states.js file still uses
.attr()
inappropriately.line 484 - disabled
line 486 - disabled
line 512 - checked
From the jQuery 1.9 change guide [http://jquery.com/upgrade-guide/1.9/#attr-versus-prop-]