Using jQuery 1.7, the views rewrite results UI does not work. Clicking "REWRITE RESULTS" expands the options, but clicking the checkbox for " Rewrite the output of this field" does not expand the options below it. Neither does the "Output this field as a link".
There are several problems with the older versions of jquery and many Drupal users would like to use jQuery 1.7. Please see here for more information, http://drupal.org/node/1386294.
One of the changes is the transition from the 'attr' function to the 'prop' function for things that are not attributes. This may be the cause of the issue.
I was unable to determine where that checkbox was triggering the display of the other field in order to solve the issue. If someone is able to point me to that spot, I can try to debug the issue.
I am using Drupal 7.12 with the newest versions of views on a fresh Drupal Install.
Comment | File | Size | Author |
---|---|---|---|
#59 | Views3.6_jQuery1.7_Accordion-on-cheked-boxes-not-works.JPG | 88.55 KB | Bernsch |
#59 | Views3.6_jQuery1.8_Accordion-not-works.JPG | 35.44 KB | Bernsch |
#43 | dependent.js_.6x.patch | 431 bytes | praestigiare |
#42 | dependent.js_.6x.patch | 431 bytes | praestigiare |
#30 | ctools-dependent-js-broken-with-jquery-1.7-1494860-30.patch | 505 bytes | gagarine |
Comments
Comment #1
dawehnerAs #dependency is actually part of ctools, move the issue, though i thing jquery update should fix these issues.
Comment #2
damontgomery CreditAttribution: damontgomery commentedOk, I didn't know it was a ctools issue. I will look more into this when I get the chance and see if I can find a solution.
dawehner, if you or anyone else can point me in the right direction, it would be greatly appreciated. I've already worked with ctools to fix a library / js loading issue discovered as a result of trying to implement jquery 1.7.
Thanks.
Comment #3
damontgomery CreditAttribution: damontgomery commentedOk, I have attached a patch which I believe fixes the issue. I will need to do some more testing, but the general idea is that the dependent.js file uses the changed 'attr' function for checkboxes and other non attribute properties.
My solution was to add in a check for the 'prop' jquery function (available in 1.6 and later) and use that if possible. if not, fall back on .attr.
It's a somewhat sloppy solution, but it works for 1.5 and 1.7 versions of jquery.
As I mentioned, I will investigate further and see if anything else needs to be done.
Thank you.
Comment #4
merlinofchaos CreditAttribution: merlinofchaos commentedDrupal ships with jquery 1.4.
You cannot supply a patch that will break with the version of jquery shipped with Drupal. That's insanity.
Comment #5
merlinofchaos CreditAttribution: merlinofchaos commentedIf I could read while drinking I would've realized that the patch already attemtps to take that into account. Sorry! Setting needs review.
Comment #6
damontgomery CreditAttribution: damontgomery commentedmerlinofchaos,
If you have any suggestions on better methods / documentation standards, I'd be happy to give it a shot.
I was running out the door last afternoon and wanted to throw a working patch together as a starting point. I identified this single issue, but it may be worth combing through ctools and seeing if there are similar issues. Perhaps there is a better process for doing this.
Thank you.
Comment #7
dawehnerJust an idea: Extend jquery so it still supports attr. Actually i think this should be done by jquery update, as this will be a problem in a lot of different places.
Comment #8
damontgomery CreditAttribution: damontgomery commentedThat is a good idea, but will be a tricky beast.
I'm not sure of all the ins and outs of the attr and prop functions. There are some cases where we may be able to simply use the other, but we'd still need to parse the functions and figure out if this is the case. Really, it should be part of jquery itself. If it can't use attr, it should automatically pass the call to prop.
Because of these issues, it may make more sense to patch a bunch of things individually. Especially considering we may be able to patch a few widely used modules such as ctools and fix the majority of the issues.
The jquery_update discussion took the direction of moving all update concerns to the individual modules, but there may be a thread over there (and if not, you can create one) to further the discussion of creating a jquery extension.
I'm not sure if the two approaches need to be exclusive, but I'm not sure I can approach the extension option myself at this point.
Comment #9
gagarine CreditAttribution: gagarine commented@dawehner I created an general issue #1498858: [meta] attr and prop
Comment #10
dawehnerLet's mark this one as duplicate because i think this is the only proper way to actually support jquery 1.7
Changing every instance on .attr would require a whole bunch of work, especially if it could be fixed in somehow a simple line.
Comment #11
gagarine CreditAttribution: gagarine commentedI reopen after my investigation http://drupal.org/node/1498858#comment-5792550 we don't need a version switch . We only have to change the line.
Comment #12
damontgomery CreditAttribution: damontgomery commentedHere is a patch which was tested with 1.5 and 1.7 that does not use a check but makes sure that val is set as a boolean explicitly for all values of jQuery. This should not break in the future. I would like to submit this as a patch to be integrated regardless of other jQuery update plans.
Please let me know if we want a different approach.
Tested on Drupal 7.12 and newest copies of ctools, views, and jquery_update on a new install.
Thank you gagarine for your work and for pinpointing the cause of the issue.
Comment #13
nod_Like it says in the comment, just get rid of
|| 0
it doesn't make sense. The return value is already truthy or falsy, you don't need to cast to boolean and you're not using it correctly eithernew
should be involved when you do that.Comment #14
damontgomery CreditAttribution: damontgomery commentedHere is an updated patch with recommendations from nod_ in #13. Getting rid of || 0 as suggested does not work when I tested it. Feel free to test it however you like and submit a patch.
The specific use case is toggling on and off several times the "Rewrite Results" option in views.
My patch works and I don't see any issues with the code. It is backwards compatible and forwards compatible and ensures that the function in question returns the type of variable it was returning in older versions and exactly what is expected elsewhere in the code. If you want a "better" way, please submit the better way.
Comment #15
damontgomery CreditAttribution: damontgomery commentedHere is the patch,
Comment #16
tim.plunkettAs pointed out in #13, there is no need to use
new Boolean
like that.Comment #17
damontgomery CreditAttribution: damontgomery commentedWhy don't you both either
1. Submit a tested patch
2. Provide a more detailed and tested explanation than "this is un-needed".
I tested my patch and it works. I tested the "better" solution and it did not work. It doesn't seem like either of you have done those things and you aren't providing better solutions.
Any reason not to use the fix if no one is willing to make a "better" solution? Does my fix introduce any issues?
Comment #18
tim.plunkett@pandaeskimo, two reasons.
1) I trust the word of the core JS maintainer
2) I was just reading JavaScript: The Good Parts (the author also wrote http://jslint.com) which in Appendix B.11 suggests never using
new
in JS.Comment #19
nod_I'm glad you're submitting a patch and making this move forward. The thing is I have limited time to address this kind of issues and, as you can see, sometime I don't do a good job of explaining all the details.
If removing the
|| 0
does not work, it means the code using this is relying on a wrong assumption about javascript. I did not have time to look more into it to see where things goes wrong. The fix you submitted feels like a hack, the proper way to address this issue is understanding why the0
is needed and why havingundefined
does not work.What I meant by my comment was that it would be better for everyone, especially for people maintaining the code, to understand the issue, and not just submit a quick and dirty fix.
Comment #20
damontgomery CreditAttribution: damontgomery commentedThanks for the responses. I still think that if the only person with the time to fix the problem does not have the knowledge to provide a "better" solution, the fix should still go through. That person is me in this case, but I would gladly have someone else pick up and provide the better solution.
Here is the code, followed by some of my observations that may point people in the right direction. If you want to provide any suggestions, I may be able to investigate and test with some help,
The issue here is that 'val' is not only evaluated, but returned as part of the first function 'getValue'. It is then checked using an inArray function in the 'setChangeTrigger' function.
I think important to note is that it is checked if the value is null and also if the value is in that array.
I'm not really sure what's being done there.
What I do know is that the old jquery would always return a boolean with the 'getValue' function, however, the new one returns 'checked' in some situations. I do not understand how the '|| 0' worked and I do not believe that was the cause of the problem. I believe the '|| 0' was just a red herring that happened to sit in the same spot where we were encountering the issue.
Comment #21
nod_well one obvious thing is:
javascript funky coercion strikes again. That's why strict equals are safer to use.
Now I'm not quite sure what kind of values can be in
Drupal.settings.CTools.dependent[id].values[bind_id]
so theinArray
, might be a problem as well.So more digging is required to get what's happening here.
Comment #22
damontgomery CreditAttribution: damontgomery commentedHere is what I have found. There is a discrepancy in way the dependent.js is evaluating values and the way forms are reporting the types of values that count as "true".
Please see the comment at the top of dependent.js copied below,
This is the clue to the line below,
The issue is multi-part and I don't know the best solution. Let me work backwards in that line to make it clear what is happening.
1. In older versions of jQuery, val evaluates to TRUE or FALSE for checkboxes.
2. The inArray() function used checks each value in the array with something like, ($element == $val) and not ($element === $val).
3. The dependent array for these checkboxes in the view is a single element of "1".
4. Since this is a poor == and not ===, (1 == TRUE) returns true.
5. In new versions of jQuery, checkboxes are returned as 'checked' or undefined.
6. Niether 'checked' nor undefined == 1.
7. The expression is always false.
There are two solutions. 1 is to fix every form that uses this functionality to include in the array 'checked' which would then evaluate to true. The other is to continue to output 'checked' values as the old way, TRUE / FALSE. This is what my patch does before and would work for all current forms that use this dependent.js functionality.
'val != null' and the earlier 'val == null' should be rewritten 'val !== null' and 'val === null' either way.
I'm not sure where the arrays are set for the forms, but there lies the problem. If someone wants to patch all those locations, that may be the better solution. In the meantime, it may be best to use the 'hack' of outputting 'checked' as TRUE and undefined as 'FALSE'.
Comment #23
JamesK CreditAttribution: JamesK commentedThe data for the checkboxes expects the val variable to be T/F, so all we need to do is just make it T/F.
var val = $(trigger).attr('checked') || 0;
should becomevar val = $(trigger).attr('checked') ? 1 : 0;
Comment #24
damontgomery CreditAttribution: damontgomery commentedJamesK,
1 is not the same as TRUE. 1 is an integer / number, TRUE is a boolean.
There are several issues and two possible solutions I proposed in #22. I don't have the time or knowledge to tackle the complex task of correcting all of the times people set "accepted" values as "1". I believe we should modify the code so that the function outputs a boolean TRUE / FALSE just as it used to, but that solution was blocked by nod_ and tim.plunkett.
I did some experimenting with the PHP in_array function using both the strict and non-strict option.
I've attached a table of the results. The arrays used are on the left column and the values tested are in the first row. From this table, you can see that without Strict, the in_array function gives very weird results and is part of the problem here. Some people have put the list of valid "checked" options as "1" and you can see that "1" is true for "1" or TRUE, but not for other things like "checked". This is because a string is converted to a number based on its first character. A string starting with a number like "5feet" is converted to 5, but since "checked" does not start with a number, it is converted to 0.
People could use the value TRUE which will take any truthy values.
Honestly though, we should use the strict flag always and people need to create an array of accepted values that actually matches these.
This feeds into the "doing it right" long solution that may not be feasible.
Hopefully this helps clarify the issue a bit.
Comment #25
gagarine CreditAttribution: gagarine commentedI made a lot of experiments about the difference between 1.4 and 1.7.
See http://drupal.org/node/1498858#comment-5792550
To avoid problem I suggest to always use attr with boolean (true or false).
I quickly edit the patch from #15 (don't have a git ready)
Comment #26
damontgomery CreditAttribution: damontgomery commented#25 seems fine to me.
To give you an idea of why I had the second part "|| $(trigger).attr('checked') == 'checked'", I believe it was because I was originally not using the method of casting to a boolean and wanted one of the values to always be TRUE.
#25 is simpler and more straight forward.
I would even consider the following change (since some people don't understand this If / Else syntax,
Comment #27
gagarine CreditAttribution: gagarine commented@ pandaeskimo + 1 to avoid the short syntax. I personally hate it, I think it make the code harder to update..
I assign to me so I have less chance to forget about it, but feel free to propose a patch from #25 and the proposition of #26 using full if syntax.
Comment #28
Zsuffa Dávid CreditAttribution: Zsuffa Dávid commented#25 Works for me.
thx
Comment #29
JamesK CreditAttribution: JamesK commentedRegarding short syntax, it is used throughout Drupal's existing code, so it is fine to use it. In this case, we should use it because the line we are patching already uses it, and the patch will make more sense.
Comment #30
gagarine CreditAttribution: gagarine commentedHere a patch. Same than #25
Comment #31
JamesK CreditAttribution: JamesK commented#30 fixes the issue with 1.7 perfectly. I've been using it in production since May.
I would RTBC it but I haven't tested it with 1.4
Comment #32
Macronomicus CreditAttribution: Macronomicus commentedWorks for me...
please put it in :)
Comment #33
dawehnerWhy not just use $(trigger).attr('checked') as it is a boolean already?
Comment #34
tim.plunkettIt could be the string 'checked'. I think this is correct.
Comment #35
damontgomery CreditAttribution: damontgomery commentedYes, the whole point here was that attr / prop of check boxes has changed in different versions of jQuery. The patch checks for a truthy value and sets the result as a boolean which is explicitly required by other code.
Thanks for the work guys!
Comment #36
gagarine CreditAttribution: gagarine commentedI wrote a small documentation about it https://drupal.org/node/1720586.
And I guess is time to maintainer to commit that now.
Comment #37
JamesK CreditAttribution: JamesK commented#30: ctools-dependent-js-broken-with-jquery-1.7-1494860-30.patch queued for re-testing.
Comment #38
ParisLiakos CreditAttribution: ParisLiakos commentedthanks, works for me as well:)
Comment #39
merlinofchaos CreditAttribution: merlinofchaos commentedCommitted and pushed.
Comment #40
宁皓网_王皓 CreditAttribution: 宁皓网_王皓 commentedWorks for me , Thanks.
Comment #42
praestigiare CreditAttribution: praestigiare commentedSame patch, rolled for 6.x if anyone needs it.
Comment #43
praestigiare CreditAttribution: praestigiare commentedSorry - fixed.
Comment #44
stevenx CreditAttribution: stevenx commented#30 worked for me. thanks.
Comment #45
joehudson CreditAttribution: joehudson commentedworks for me too.
Comment #46
airstarh CreditAttribution: airstarh commentedHello
Patch #30 worked on Drupal 7.16
But after updated to 7.17 it happens again!!!
My configuration is:
Drupal core 7.17
Views 7.x-3.5
Ctools 7.x-1.2
Jquery update 7.x-2.3-alpha1 (I us library 1.7, because when I use 1.8 views constantly offer me download a file...)
Comment #47
airstarh CreditAttribution: airstarh commentedHm. Nothing works without patch #30
But still it becomes impossible to use js in Views with jquery higher 1.5
(((
So the only way I see - disable js in Views (((
I see a great damage here
Comment #48
cooldeeponline CreditAttribution: cooldeeponline commentedPatch #30 worked for me:
My configuration:
Drupal core 7.18
Views 7.x-3.5
Ctools: 7.x-1.2
jQuery Update: 7.x-2.3-alpha1+0-dev
Thanks!
Comment #49
kaidjohnson CreditAttribution: kaidjohnson commentedPatch #30 worked for us too.
Drupal 7.19
Views 7.x-3.5
Ctools 7.x-1.2
jQuery Update 7.x-2.3-alpha1+0-dev, running jQuery 1.8
Thanks!
Comment #50
m1n0 CreditAttribution: m1n0 commented#30 worked for me also.
Thanks!
Comment #51
woop_light CreditAttribution: woop_light commentedbingo!
Drupal 7.19
Views 7.x-3.5
Ctools 7.x-1.2
jQuery Update 7.x-2.3-alpha1+0-dev, running jQuery 1.7
Thanks Pandeskimo!
Comment #52
christoph CreditAttribution: christoph commentedFantastic - worked for me too
Drupal 7.19
Views 7.x-3.5
Ctools 7.x-1.2
jQuery Update 7.x-2.3-alpha1+0-dev, running jQuery 1.7
Brilliant.
Comment #53
Exploratus CreditAttribution: Exploratus commented#30 Worked for me. sub fields for labels and such weren't appearing in views. Now they do, using jquery 1.7.
Comment #54
joshuautley CreditAttribution: joshuautley commented#30 worked for me. Thank you.
Comment #55
Stathes CreditAttribution: Stathes commentedApplied the patch in #30
In addition:
Drupal 7.19
Views 7.x-3.5
Ctools 7.x-1.2
jQuery Update 7.x-2.3-alpha1+0-dev, running jQuery 1.7
Still, no luck when I try to rewrite output of a field or make field a link drop down works... nothing happens.
Comment #56
merlinofchaos CreditAttribution: merlinofchaos commentedThe patch in #30 is already committed (months ago) and working for dozens of people. There's no value to re-opening this issue. You very likely have a different issue.
Comment #58
dongtian CreditAttribution: dongtian commentedApplied the patch in #30
In addition:
Drupal 7.20
Views 7.x-3.5
Ctools 7.x-1.2
jQuery Update 7.x-2.3 running jQuery 1.7
It works for me.
Thanks a lot.
Comment #59
Bernsch CreditAttribution: Bernsch commentedI have the same problems with the views "rewrite results" and "style settings" UI when i am using jQuery 1.7 or 1.8! (see Screenshots)
I use Drupal 7.21 and folowing Modules:
The patch in #30 works for me with jQuery 1.7
(INFO: With jQuery 1.8 doesn't works.)
Comment #60
luthien CreditAttribution: luthien commentedI have the following setup that still does not works, it looks like the patch was never committed for Ctools 7.x-1.2.
Drupal 7.21
Views 7.x-3.6
Ctools 7.x-1.2
jQuery Update 7.x-2.3 running jQuery 1.7
Applying patch #30 (manually) solved the problem.
Comment #61
achtonThis fix really was commited and is present in CTools 1.3 (look!).
EOD.
Comment #62
jawad.shah CreditAttribution: jawad.shah commentedRef #30
Patch works for me. Thanks gagarine.
Comment #63
hockey2112 CreditAttribution: hockey2112 commentedUpdated Ctools and the issue is fixed for me. Thanks!
Comment #64
sundevil CreditAttribution: sundevil commentedTo allow View > Field > Style Settings > Customize label HTML checkbox to expand.
Alternative FIX:
Optionally select a different version of jQuery to use on administrative pages.
Modules > jQuery Update > Configure > Version Options > Alternate jQuery version for administrative pages > 1.5
Comment #65
jwineichen CreditAttribution: jwineichen commented#64 alternative fix worked for me. Thanks, sundevil!
Comment #66
asb CreditAttribution: asb commentedThe issue still exists for me with ctools 7.x-1.5 from 2014-Nov-19 when running JQuery 1.9 as default.
The suggested workaround from #64 resolves the issue, but the incompatibility seems not to be fixed in current ctools. Any comments?
Comment #67
owenpm3 CreditAttribution: owenpm3 commented#64 changing jQuery version for administrative pages worked for me. Thanks!
Comment #68
othermachines CreditAttribution: othermachines commentedFYI: If using jQuery >= 1.9 and this is an issue for you, it's probably because of this: Issue #2416689: jQuery .attr() deprecated/removed for use with "checked" and "disabled" properties
I've confirmed that changing line 100 in js/dependent.js:
to this:
.. got the checkboxes working again. The former was implemented when this issue was fixed. The latter is suggested by the patch at the aforementioned issue.
Comment #69
othermachines CreditAttribution: othermachines commentedComment #70
bpunzalan CreditAttribution: bpunzalan commentedI had this problem. I am using jQuery v 1.10 and my default jquery for administrative pages is defined by jQuery. I solved it by changing the
Alternate jQuery version for administrative pages
in jquery_update module toDefault (Provided by Drupal)
. I think it will use jQuery v 1.4 in misc/jquery.js for administrative pages because i tried to unset jquery files before and jQuery v 1.4 automatically occur in the page by default. Just my observation, correct me if i am wrong. I'm new in drupal. Thanks.Comment #71
ttom CreditAttribution: ttom commentedThanks punzalan25brian (#70). I did the same and the problem is gone.
Comment #72
stanoreke CreditAttribution: stanoreke commented#70 works for me thanks.
Comment #73
digitalpaperbacks CreditAttribution: digitalpaperbacks as a volunteer commented#64 worked! changing jQuery version for administrative pages. Thank you!