Closed (fixed)
Project:
Hierarchical Select
Version:
7.x-3.0-alpha9
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
15 Jun 2010 at 20:31 UTC
Updated:
18 Dec 2014 at 16:40 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
wim leersI'm not sure what you mean (maybe it's because I'm tired, in which case I apologize). Can you please rephrase that?
Comment #2
TonyK commentedThe problem:
There is a call to Drupal.attachBehaviors in Drupal.HierarchicalSelect.update function (hierarchical_select.js):
Clearly, the first argument passed to Drupal.attachBehaviors function must be a DOM element that was inserted by JS. But Drupal.HierarchicalSelect.context is a function, not DOM element.
The question is, how this code is going to work? Maybe it's better to replace it with
?
Sorry for my English.
Comment #3
TonyK commentedComment #4
wim leersHm … you might be right here, but it's always worked like this for me. Can you do some testing to confirm your hypothesis?
Comment #5
wim leersAny news?
Comment #6
Isostar commentedsubscribe
Comment #7
glennpratt commentedI'm having this issue as well, Hierachecal Select breaks other Drupal behaviors on the page if they implement context.
See: http://api.jquery.com/jQuery/
A simple search and replace stops breaking other code, but then HS stops working.
Comment #8
glennpratt commentedActually, I take that back, HS works ok for one request, then goes into nojs mode with this change:
Drupal.attachBehaviors(Drupal.HierarchicalSelect.context());Comment #9
glennpratt commentedFor more evidence, my code should only update it's elements if they are in context, but when HS fires attachBehaviors, it finds anything in the document. If I actually call context as a function, it doesn't find my elements as I expect.
Test code in my behavior:
Result:
Comment #10
wim leersYou're calling context(), not Drupal.HierarchicalSelect.context().
Comment #11
glennpratt commentedSorry if I was unclear.
The previous test code was in MY Drupal.behaviors function, I don't plan on calling anything to do with HS in my code! I've updated it to be more clear.
HS is calling it on line 556:
The problem is, the argument is a var defined as a function and passed to my code...
jQuery doesn't expect context to be a function and appears to ignore it or cause random problems (on some browsers my behavior would just stop working after an action happened in HS.
Here is my fix for my code, I tried to fix HS, but it wasn't straightforward to fix.
Comment #12
wim leersVery strange, that used to work just fine in the past. But, this should be easily fixable. Please review the attached patch.
Comment #13
glennpratt commentedLooks good in some quick testing. I tried and failed to do what you did in your patch, probably missed a line...
This will be included with code going into testing soon, so I should know pretty quick if it breaks.
Thanks so much.
Comment #14
wim leersOkay, great! Don't forget to let me know if this works! :)
Comment #15
glennpratt commentedWell, I'm afraid cacheing may have gotten the better of me, now I'm seeing the 'You don't have Javascript enabled' message.
I'll try to test it on a clean environment in the next couple days.
Comment #16
wim leersOkay, thanks! Keep us posted :)
Comment #17
Georgique commentedPlease take a look on following issue, seems that our bug needs some more work: http://drupal.org/node/1507954
Guys changed code in states.js from
to
So now it won't work correct with HS since HS context returns function instead of DOM element. I'm not sure what is the nice solution of this problem, but I have a feeling that HS context should not be function anymore.
Comment #18
Georgique commentedI think I fixed it.
Let me explain.
When we had been passing function as context, system didn't understand it so it used document DOM object instead. Also every selector in hierarchical_select.js file uses hsid, so context doesn't needed there at all. And that's why it worked.
Only the place where we need context is the following line:
Drupal.attachBehaviors(Drupal.HierarchicalSelect.context);As I explained it worked before, but behaviors were attached for the whole document object. This is not what we want, so @glenpratt had tried to change it to
Drupal.attachBehaviors(Drupal.HierarchicalSelect.context());but this was also wrong because:
1) Drupal.HierarchicalSelect.context() returns array, not single element;
2) It returns .hierarchical-select-wrapper element, but look at the following code:
$('.hierarchical-select-wrapper:not(.hierarchical-select-wrapper-processed)', context) won't work since context is .hierarchical-select-wrapper.
So I changed it to the following code:
Seems now it works fine, it shouldn't break other fields, it works ok with several hs fields on one page.
Review needed and I guess it would be nice to commit this and introduce new version of hs asap.
Comment #19
kaizerking commentedI have applied this patch to get two HS fields on the same page.
it did not work.
When i create initially it is available and when i try to edit and save it wont save
Comment #20
Georgique commented@kaizerking I can't reproduce, on my machine it works fine. Can you debug?
Comment #21
sreynen commented#18 is on the right track, but has two problems. First, the patch includes a lot of changes that don't appear to have anything to do with this issue. Second, the code has changed somewhat since the patch was posted.
The code now passes context()[0], a single element, which is close, but still wrong. As #18 explains, this is passing the HS element, but jQuery doesn't find elements within themselves.
Attached patch applies the solution outlined in #18, passing in the parent instead of the element itself.
Comment #22
thomas73 commentedThanks, this patch seems to resolve my problems.
Comment #23
ryanoreilly commentedAlso confirming #21 fixes Alpha-6 bug.
Comment #24
mesh commented#21 makes alpha6 at least usable. However, node forms still get auto-submitted on creating new terms.
Comment #25
sreynen commentedI didn't see any mention of creating new terms in this issue. Is there a separate issue for that? If not, let's make one and fix that separately. With 187 open bugs in this project, we can't solve them all in one patch.
Comment #26
kars-t commentedI apllied this to a simple D7 installation and the JS problems are fixed with this patch.
Comment #27
ssoulless commentedPlease let us know if you open a new issue for fix the bug while creating new terms
Comment #28
wim leersThanks, committed!
http://drupalcode.org/project/hierarchical_select.git/commit/aac15cd
Comment #29
cravecode commentedThe patch from #21 is still not ideal and only works if one Hierarchical Select is on the page. It's using the Context (Drupal.HierarchicalSelect.context) and selecting the first element to re-attach the Drupal behaviors thus skipping over any additional Hierarchical Selects in the form. I believe we should be grabbing the wrapper of the initiating element.
Patch Summary:
Replace:
Drupal.attachBehaviors(Drupal.HierarchicalSelect.context().parent()[0]);With:
Drupal.attachBehaviors($('#hierarchical-select-' + hsid + '-wrapper').parents('div.form-type-hierarchical-select')[0]);My submitted patch is for the 7.x-3.0-alpha7 tag.
This patch has been tested with:
Comment #30
puppyman commentedhttps://drupal.org/node/828418#comment-7939351 Works for me! Had to manually patch, however.
Comment #31
ssoulless commentedWorks pretty well, i did not wanted to post another comment here due to Im using simple hierarchical select, but the error that you say is happening, and the current "comitted" patch is not enough, i tried your patch "had to manually patch" and it works pretty great!
Comment #32
cravecode commentedThe reason a manual patch is required is because it was created for the latest tag (7.x-3.0-alpha7) in the Git repository. If you aren't cloning from Git and instead downloading via Drush or from drupal.org/project/hierarchical_select you're not getting the latest release, you're getting the 7.x-3.0-alpha6. Alpha7 looks like it was created specifically to include the #21 patch from this issue.
Hope that helps!
Comment #33
wim leersI am very puzzled why alpha 7 does not show up. I *did* create it, immediately after pushing the commit in #28. WTF!? I even viewed the project release node after creating it IIRC. This is extremely bizarre. Looks like something went wrong on drupal.org… :(
But apparently it's a good thing that the alpha 7 release didn't succeed on d.o, because just like in alpha 6, a fully validated patch (i.e. RTBC'd in #26) turns out to be problematic.
However, the patch that's been RTBC'd this time is also fundamentally flawed IMHO; it no longer calls
Drupal.HierarchicalSelect.context(), which is a fundamental change that should not have to happen. However, sincet there are 3 independent confirmations that it works, I'll commit it anyway. The D7 codebase is a mess already anyway. Better to have a working mess.http://drupalcode.org/project/hierarchical_select.git/commit/b3e34d9c2a0...
Comment #35
swarad07I applied the patch in #21 on alpha 6, it works only for the first HS field.
I have a CC with two HS fields, on the node/add or node/edit page which ever field is the first field the JS works for it. For the second field the JS breaks and shows the update button.
Any idea on how to fix this ?
Comment #36
swarad07Sorry guys my bad! I was looking at the wrong code.
Comment #37
sylus commentedAttaching a patch that will work with Alpha 6 for drush make since Alpha 7 wont work.
Comment #38
ssoulless commentedAwesome finally a patch that really works!! thanks Sylus
Comment #39
itangalo commented+1 for people working on this issue. Makes me happy.
Comment #40
darktek commented#18 just worked really great thx so much !!
Comment #41
kevin.dutra commentedHere's a patch ported back to D6.
Comment #42
heddnAdded a follow to this issue #2301699: Timing to release new hs version for D7.
Comment #43
ssoulless commentedThe patch works for Drupal 6 correctly. Nice...
Comment #44
stefan.r commentedD6 patch committed, thanks!
Comment #45
stefan.r commentedComment #46
calculus commentedI am not sure that i have to open this issue again, but the "update button" problem remains after updating to 7.x-3.0-alpha8
Should i comment there? #2000762: Problem after the last update 7.x-3.0-alpha6, it shows me a button 'Update' while selecting
Comment #47
stefan.r commentedYes, that's fine, these fixes are not in alpha8 yet.
Alpha8 is just alpha6 with a security update. We'll do an alpha9 soon.
Comment #48
calculus commentedOk thank you. Any quick and dirty patch? I tried #41 and worked locally but not on live server...
Comment #49
fredfab commentedThanks you all for your works on this issue ! We clearly need the same fix for Alpha8 since it's a security release.
Comment #50
nicolas bouteille commentedShouldn't this issue version be 7.x for now?
Comment #51
stefan.r commentedReopening this issue in case anyone has a better solution than the latest patch, which still has the flaws pointed out in #33
Comment #52
pianomansam commentedApparently this was fixed in 7.x-3.0-alpha9 but not updated to reflect that.