Closed (fixed)
Project:
Hierarchical Select
Version:
6.x-3.x-dev
Component:
Code - Taxonomy
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
11 Oct 2009 at 14:12 UTC
Updated:
15 Mar 2010 at 01:00 UTC
Jump to comment: Most recent file
Comments
Comment #1
wim leersI have no idea. I won't track down this bug (I don't have time for it). Hopefully somebody else will.
Comment #2
bibo commentedThis simple patch for "hs_taxonomy.module" should fix the problem. Basically it's calling the form_alter of "content_taxonomy" after it has done it's normal form altering.
I guess setting the module weight of content_taxonomy to be greater than "hs_taxonomy.module" would have worked also.
Comment #3
wim leersThe weight of content_taxonomy or hs_content_taxonomy?
The solution you proposed may work, but is unacceptable. Changing the module weights sounds much better :)
Comment #4
bibo commentedEventhough the problem concerns the HS+CT integration, I didn't mean altering hs_content_taxonomy, at all.
I did some fast tests, and both my patch, and setting "content_taxonomy" weight to 3, do work.
But I guess altering a 3rd party module's weight is not good solution.
On the other hand, you could set "hs_content_taxonomy" and "hs_taxonomy" a lower weight than content_taxonomy
(which is a default 0), so they run before content_taxonomy... But if their weight is less than 0 they also run before
lots of other modules that probably should run before them (such as taxonomy, forum etc).. or am I mistaken?
There must be a reason why "hs_taxonomy" weight is 2 instead of 0? Altering any weights might mess up
lots of things with a million other modules..
I know you're well aware of all this, but do you know if there's an elegant solution to go around this -
without altering the 3rd party content_taxonomy weight?
I understand running another module's form_alter would usually be a big no-no... but could you reconsider this option
given the following additional information :
content_taxonomy-package has several modules with various form_alters, but the form_alter of the base module
content_taxonomy only hides the taxonomy fields (and more specifically, only the ones that are handled as CCK-fields).
The full form_alter looks like this:
A third solution would be to just copy paste that whole code at the end of hs_taxonomy_form_alter(), instead of my suggested patch:
I know the form_alter of content_taxonomy might change later, and could cause problems then.. still those few lines looks rather clean compared to the above, right :p?
Comment #5
bibo commentedOk, so how about this patch?
Code-wise is pretty much the same, except I just duplicated code from content_taxonomy_form_alter() to be run
in hs_taxonomy.module:s form_alter (after the normal altering). The only downside should be code duplication.
The patch is made against the latest dev version (2009-Oct-19).
Comment #6
TripleEmcoder commentedWhat is the core issue here? Is it HS that makes the core taxonomy fields reappear (why? could it perhaps detect that content_taxonomy is active?) or is it content_taxonomy that does not recognize core taxonomy fields altered by HS as, well, core taxonomy fields (why? wouldn't it still be better to add support to recognize them there than to duplicate code?)?
Comment #7
bibo commentedIn short, yes. Or rather, its not the core fields reappearing, but the overridden HS values.
If you look at the patch this line does exactly that:
The new code is only executed if hs_content_taxonomy is installed and enabled. And since hs_content_taxonomy depends on content_taxonomy (and hierarchical_select and hs_taxonomy), those
have to be enabled as well.
Content taxonomy can't recognize things set by HS, since those will be only added after content_taxonomy has run its code. That is because hierachy_select is run after content_taxonomy, and also hs_taxonomy (weight ) is run later. hs_taxonomy is also even later than hs_content_taxonomy. Setting content_taxonomy module height higher would make it run later and then automatically hide the taxonomy fields set by hierachy_select. However, this could mess up pretty much anything, and is imo not a solution.
The code duplication from content_taxonomy_form_alter() is not that much code, and it simply works.
Altering module weigths can get very tricky. For example, check this install script of hierarchical_select:
memfis, could you please try my patch and report if it works for you too, so we maybe get this committed to the module?
Comment #8
hefox commentedExperiencing the same issue
My views are going to be ...picky but here's a few opinions on this issue (not the patch specifically, the issues leading to it):
In some respects, a parent module should not know about a sub module.
Also, the hs_taxonomy module changes things that people wanting to use hs_content_taxonomy might not want changed, like parent addition on term edit pages, so having it as a sub module, meh!
Duplicate code is generally a bad idea (ie fix something somewhere, forget it elsewhere.. eek!), but making a general shared function is cool:>.
Probably a bad idea to change unrelated items; I'm weary about how hs_taxonomy blows up the taxonomy form element and re-does it; that is somewhat the issue here I think? If instead it only blew up the taxonomy for the vocabularies it was enabled for and checked the elements were in the forum still, the problem would be gone right?
current workflow:
1) remake taxonomy (which also is problematic for other modules trying to effect taxonomy element, like modules that also change one of the input elements. Makes the weight issue so more messy?)
2) add in some hs changes
instead
1) all hs content_taxonomy code in it's own module only effecting itself.
2) add in some hs changes to relevant form elements based on vocabulary
3) check if fieldset should be removed from taxonomy element.
I'm willing to try making patches, but I'm not up on some of the issues that lead to the current issue ... Or alternitly bibo's patch, if it works then go with that :>. (Haven't tried it, I did some simple changes since I don't have forms that both have hs_content_taxonomy field and taxonomy).
Sorry if I sounded mean/picky/etc.
Comment #9
bibo commentedI'm not sure what you mean with parent and submodules. As I see in Drupal there is no clear module hierarchy (except in this specific module name, hehe), there are just dependencies and order of execution (order being alphabetic and module weights). I guess you ment hs_taxonomy.module as sub, since it depends on "hierarchical_select" and "taxonomy" . Or hs_content_taxonomy that depends on the same + hs_taxonomy and content_taxonomy.
Hmm. Since all the hs-modules are in the same project (and package), imo they should know about each other. And most big projects support/integrate with each other also... but this isn't the problem, and a bit offtopic, isn't it?
Yeah, and that function would be content_taxonomy_form_alter(), as I suggested in my first patch. That version would limit to patch to a few lines (or only one if you want). But it's not exactly shared (or normally not supposed to be used by other modules). The function just happens to be doing exatcly what is needed, but since it is out of this module's control, and it might change, it is "unaccaptable" (or so I understood). Thus duplicating code, it's not that many lines anyway.
Since HS is actually ment to alter the form UI of the (content) taxonomies, I believe the correct way is to use the form API the way it does it. Not that I have gone through all the code.. But the logic isn't likely to change (not for this contrib module anyway :p). The module execution order is important tho, since the last one gets to override what ever it wants to.
Remake taxonomy is a typo, right? Since changing the core taxonomy is not possible. Not for Drupal 6 and not even for 7 since it's in code freeze allready :d. Ah I guess I again just didn't understand you.. or you didnt mean the module, but just the $node->taxonomy?
But I understood the latter 3 steps. If you mean that "hs_content_taxonomy" would include this code instead of "hs_taxonomy", I just realized that might also be possible. So far I thought "hs_content" weight is 2 so it runs after "hs_taxonomy_taxonomy" (weight 0), and thus also "hs_content_taxonomy" weight shouldn't be altered. But I just read hs_taxonomy_install():
Since the weight is because of "forum"-module, I assumed wrong.
So, to change this code the more logical module (and the one set as "component"), this would work: add the patch changes from the end of "hs_taxonomy_form_alter()" to the end of hs_content_taxonomy_form_alter()-function. And also add a new file: "hs_content_taxonomy.install":
I won't make a patch for that tho, unless someone wants it. Wouldnt change the result in any way.
If you don't have both in use for the same nodetype, this problem wouldn't concern you at all, right?
Would you (or just about anyone) please just try my patch - I know it works fine and causes no problems. The code just wont get any attention (=wont get committed) until someone else reports back about it working or not.
To save a bit time, I attached the patched version of the hs_taxonomy-module (for dev 2009-Oct-19-version).
Comment #10
hefox commentedHm, here's my not very tested alternate patch.
The most important part in it is I get rid of the override taxonomy selector. If one was completely overriding every taxonomy selector, that'd seem like a great thing to do, however what it ended up doing (from what I could see) is essently repeating taxonomy_form_alter.
Why I saw this as an issue
1) Repeating code makes fox a sad panda. (not that fox doesn't tend to do that, but fox is a hippo sometime, and not even a hungry hungry hippo at the moment, but a sleepy, sleepy hippo). This is problamic for when the code gets updated; vocabulary->help was updated in core (run through a special parser) but this code wasn't updated (not yelling at maintainer for that! just saying repeating code leads to this type of stuff!).
2) Bulldozers other module's code, for example i8(whatever)taxonomy, the taxonomy translate module(which basically does similar to this module on repeating code from what I could see based on lullubot api's current api of the code, again, sad panda :( ), translates help, descriptions, etc. based on settings. I see that in hs it tries to componsate for this by re-doing the translation, but it misses translating the fields I think. Then, it misses any other module it doesn't know about changes, like content_taxonomy, ie this issue).
So, if it remains as is it'll likely have to keep changing to support any other module that alters $form['taxonomy'], or some delicate balance of weighting, and also has to be kept updating if core changes again (unlikely). (can't think of any contrib modules atm. For example site specific modules people [well, i at least] use to change stuff like the fieldset name would need to be weighted above hs_taxonomy.).
So this patch gets rid of overriding taxonomy selector and instead goes through $form['taxonomy'] (still needs to be above taxonomy weight wise) to look for vocabularies to transform. (With this, idealy content_taxonomy runs before in this case, so it doesn't process anything that content_taxonomy will remove, and that is likely +1 for performance). Instead of reloading the vocabulary, it uses the form element information to set it's information, so if a module has done changes it'll be kept (needs to run after i8(whatever)taxomy also so it does it's changes because i8(whatever)taxonomy overrides the taxonomy[$vid] form also :/)
Looking at it, I suspect I'll be making a feature request about making optional overriding term edit since it's too much shared to seperate hs_content_taxonomy and hs_taxonomy, but I'll see how this issue goes.
Again, haven't tested much yet and not with i8(whatever)taxonomy, but logically it seems likely to work.
BTW, this is probably a Code -- Content Taxonomy issue, but I dislike changing other people's issues. I believe Code -- Content taxonomy is for the hs_content_taxonomy module, where the issue here is resulting form the hs_taxonomy module. (Though the issue is effecting the operations of the content_taxonomy module).
Again, sorry if I come out as picky or mean or tired as **** :>
Comment #11
TripleEmcoder commentedSo, why does HS blindly override these fields instead of looking at the $form it gets and only overriding the fields that are still there after whatever module has run before HS (like content_taxonomy).
I think this is the approach proposed by hefox, am I right?
I am definately not suggesting messing up with module weights. What I meant was something like detecting if content_taxonomy is active as a module or perhaps better an API in content_taxonomy that would give a green light for HS to override?
But it seems that this would be almost the same as detecting active fields from $form, only with less impact on interdependencies.
Comment #12
TripleEmcoder commentedHefox, I see you deleted a lot of code from HS Taxonomy in your patch ;) That amount scares me a bit, but as far as I can tell these are good changes. I didn't know that HS Taxonomy actually rebuilds the taxonomy part of the form from scratch. Your approach is much better, in my opinion it's actually the only correct and logical one. Are there any historical reasons for the current approach?
I agree that we should move this issue to Code - Taxonomony. Someone with better understanding (than me) of HS Taxonomy code should review this patch. I'll be happy to test once we agree on the approach.
Comment #13
hefox commentedOh wow, I took over 10 minutes writing my last post (based on that bibo posted during the time I was writing so I never noticed their reply).
Anyhow, bibo my later post might be more clear. Not going to answer everything cause I'm lazy and probably explained most in the last post, but this issue was effecting anyone that uses content_taxonomy weighted below hs_taxonomy since hs_taxonomy is what was creating $form['taxonomy'] (so content_taxonomy trying to remove it failed since it's added back in).
To memfis.. yes.. I did delete a lot of code; however most of the code I deleted was duplicate code directly from the taxonomy module :P.
It's in the README (also with some what I'd consider scary advice to hack core). As far as I can read on why they did it that way, it's related to performance issues. D6 provides that variable (override taxonomy whatever) for modules to be able to completely take over taxonomy form elements, which is a cool idea but when used like here causes issues with other modules expecting to be able to edit $form['taxonomy'] at a certain time and expect their edits to stay.
Comment #14
hefox commentedTried changing the title to reflect (my) current understanding of the issue.
Mostly just noting my patch needs an update run (to reset override taxonomy back to false.)
Comment #15
hefox commentedswt, and here's a patch that doesn't cause foreach loop warning; forgot to return the array() at the end of it.
Comment #16
bibo commentedHefox, your patch is pretty big (compared to the 3 new lines required for my patch to work). Unfortunately I'm working too close to a deadline to test or work with that. I'll just do this fast post.
Does your patch actually address the fieldset unhiding (that is: fix it) or not? If not, you should probably make a new issue. It's great if you can fix for example translation issues (which I haven't noticed yet, but I will also need those fixed).
Comment #17
hefox commentedYes, it fixes the fieldset issue.
This whole issue is that hs_taxonomy was remaking the taxonomy array (the fieldset). My method only edits on valid entries in the taxonomy array, not adding or removing anything, so it honors whether content_taxonomy has removed some entries.
Though, I just realized there's one issue with free tagging vocabularies (they don't have the options array, oops).; I'll look into that tonight if I have time.
Comment #18
hefox commentedJust realized that shouldn't be an issue; people should just be advised not to have their hierarchical select vocabularies set to tags also, since it's a pointless setting and it makes things more complicated.
for example, taxonomy translation, if the taxonomy is tags it won't get translated, etc.
Comment #19
TripleEmcoder commentedHow does that event work originally, I mean HS + tags taxonomy? I suppose that a feature being pointless is not good enough an argument to brake something in a patch (or not fix entirely).
I should manage to test the patch tonight.
Comment #20
hefox commentedOoo, hs_taxonomy is already setting tags to 0 when it's enabled =)! so hs_taxonomy already took care of that.
So the part in my patch that goes over the $form['taxonomy']['tags'] should be removed.
ie
All of that part, though atm it is not hurting anything, it's not needed code.
Which my alterations to hs_content_taxonomy in a different issue, making further alterations to the patch is quite.. annoying atm, so I'd be much happy if someone could make an updated patch u.u. If not I'll make it :(.
Comment #21
benone commentedHi, I did not install any patch from here.
What I did:
Taxonomy > Edit vocabulary > Content types: UNCHECK ALL
And now I have only CCK HS Taconomy field in my form.
In this field settings page there is still option - Save values additionally to the core taxonomy system (into the 'term_node' table).
So I CHECKED it.
Is the the choosen category saved also in core taxonomy table, if in Vocabulary settings there is no Content type choosen ?
Thanks
Comment #22
benone commentedI am answering to my question.
YES. In core taxonomy the category I choosed is saved. I checked the table in phpmyadmin.
So my question is - what for is Content type choosing in Vocabulary settings ?
Comment #23
hefox commentedThe core taxonomy module handles basic adding taxonomy to node forms; the per content type settings determines what content types that taxonomy will show up on.
Content taxonomy is an add on module, which expands on taxonomy, but is not necessary. It makes taxonomy a cck field with more flexibility. It hides the field that core taxonomy adds in. As you mentioned, taxonomy does not actual need the content types settings when using content taxonomy. Personally I use them anyway as description so I can easily remember which taxonomy is on which content type.
Un-checking all content type settings for taxonomy that are having troubles is a good temporary solution, but being that many people have those settings it's should be addressed. Also, in my opinion, there's certain other issues my patch is addressing with interaction with other modules (ie, translation and using other taxonomy overrides.)
Comment #24
benone commentedRight. Temporary solution. Anyway, I will try to install your patch to have it done in a proper way.
Could you point me a link to the patch ? I don't know which one from here is the last , working version.
Many thanks for your explanation.
Comment #25
hefox commentedNewest patch
While editing out the tags for each I also made some other changes; instead of creating a new forum element I use a old one and just change the relevant settings with the intent to save any other changes a module may want to add.
benone, you may unfamiliar with testing patches, etc., you may want to wait till the maintainer weights on my patch . My patch is sort of a big change; or rather, it removes a lot of code and does thing a bit differently then the original module. But if you want to test it, test away :).
Comment #26
hefox commented(Oh right, and if anyone does test it, remember to run update 2 under update.php for hs taxonomy).
Comment #27
benone commentedOk, I will wait. Thanks.
Comment #28
wim leersI read through all of the comments above. At #10, things started to go wrong, unfortunately. hefox had noticed the use of
taxonomy_override_selectorand assumed this was an ugly core hack. I agree that it looks like it, and in a sense it *is* a core hack. However, it's an officially supported feature of core, but unfortunately an undocumented one.However, you can find this section in the README:
I fully agree that the code is ugly, but there is just no other way. catch (and I think also pwolanin) opted for this method at the end of the D6 cycle to at least make it *possible* to make this more scalable without hacking core, at the cost of copy/pasting a ton of code.
So this approach will absolutely not go in, for scalability reasons.
Next is the module weight approach, which also seems to have problems.
Finally, there is the initial approach suggested by bibo (the patch in #2). While it's not very elegant, it's the most elegant solution of all and gets the job done well.
I'd like your feedback before I commit the patch in #2.
P.S.: hs_content_taxonomy depends on hs_taxonomy because it reuses hs_taxonomy's implementation of the HS API. I.e. it reuses it exactly to prevent code duplication.
Comment #29
hefox commentedPatch #2 will work with some modifications; it should be module_exists('content_taxonomy') instead of module_exists('hs_content_taxonomy'); the issue is not due to hs_content_taxonomy :P.
As stated in one of my comments, I did read the README and understand why it was done as it was done, but see it as problamatic. (Also, I realized why the hs_taxonomy was required, as I may have said.).
The issue I have with patch #2 is that it is content_taxonomy support specific; other module that interacts with the $form['taxonomy'] array and is weighted before hs_taxonomy it's changes will be lost, which may lead to more issues and frustrations later on.
For example.i18ntaxonomy I noticed there was some calls to fix label
ie. from hs_taxonomy
However i18ntaxonomy also translates the terms of the vocabulary. As far as I can tell, (I do not use i18ntaxonomy but looked it up when looking over the code; I spent a bit looking into this...) hs_taxonomy does not address this since it ignores the current $form['taxonomy'].
Upping the weight if i18ntaxonomy would not work since it also remakes the taxonomy array.
Another example; I know a lot use little sub modules to edit information here and there. For example if I wanted to change the taxonomy description per content type, I'd do it in a site module and expect it would work, then get quite confused when it didn't work since I wouldn't know hs_taxonomy was doing this (Readme? Who reads those! Or rather, who remembers what they've read months later when trying to debug something seemingly unrelated.).
Then there's also the issue with hs_taxonomy did not get updated when core taxonomy got patched in the duplicate code, which is generally the issue with duplicate code.
Long story short: Patch 2 works, but (in my opinion) there is an issue with flexibility with other modules that it does not address. Is the scalability issue worth it?
Also, how much is saved by doing it that way?
I'm probably missing something; Most of the information is reused; no additional database calls, just a foreach loop and some edits to the array. (Could even get rid of the function calling, by putting all the changes in the foreach loop).
Comment #30
wim leersYes. It can make a world of difference on sites with multiple thousands of terms.
Because over the taxonomy_override_selector variable, *only* HS generates a form item. In Drupal 5, or without that variable, Drupal core would first generate a normal select with as many terms as there are in the vocabulary. Say 10,000. Clearly, that costs a lot of time to generate. Then HS comes along and simply *deletes* the select. I.e. all the work core did was pointless, completely in vain. That's why it's absolutely necessary to keep on using taxonomy_override_selector.
Comment #31
wim leersAnother thought: changing the module weight was necessary due to #475784: #options set by forum module, this causes a validation error. What you could do, is change the weight back to 1 and then unset #options from within HS itself, at the end of the #process callback. Would that solve all problems?
Comment #32
hefox commented(oh the forum module... only site I've used it on I ended up uninstalling it after a week. It also has other issues like not checking if body is set for the content_type ='( )
(Whatever way, should also do the changes introduced to taxonomy module .. in taxonomy_form_alter? back in.. 6.12? 6.13?. I believe an xss_ something call around vocabulary help or something
http://cvs.drupal.org/viewvc/drupal/drupal/modules/taxonomy/taxonomy.mod...
).
Do you mean set the weight back to 0?
The i18ntaxonomy module is still problematic to, since it's overriding the taxonomy array also IF taxonomy array is set. However, I think the answer is more likely to be in patching that module than this since:
It's is producing those scabilities issues that are the reason for taxonomy_override x2 I think; it's not using the variable from what I can see based on the install hook), so alone or with hs_taxonomy, it's going through the terms (again).
Simpliest way is to ask it to honour that variable and not run when it's set; an until then, weight it before hs_taxonomy.
A perhaps better solution would be to ask it to stop overriding the forum with values it does not need to change; ie #type.
The advantage to that is that could remove some of the i18ntaxonomy support from the module since i8ntaxonomy would be taken care of (other than translating terms, which I finally noticed you are doing -- sorry for not noticing that) like vocabulary names. Also, if instead they used the existing #options it'd deal with the scability issue more I believe.
So the patch would be:
Weight hs_taxonomy to run around the time taxonomy would run; my guess is -1. taxonomy and content_taxonomy are both weighted 0; taxonomy runs first due to the asc weight, asc filename in module_list (just looked that up). So -1 would run it before content_taxonomy and other modules.
Reset the options. Resetting options sounds like a good idea anyway in case other modules create similiar changes:).
Deal with i18ntaxonomy somehow.
filter $vocabulary->help
Comment #33
wim leersYes.
Interesting reasoning, I hadn't considered that. That'd be nice. But it's unlikely that it will happen. If you'd get those patches in though, I'd be very grateful :)
Right! It should be -1, not 0.
Thanks for looking into this! :) Care to roll the necessary patch(es)? :)
Comment #34
hefox commentedBah! I8ntaxonomy needs a 5 weight due to it's interaction with views in the latest dev (which would override hs_taxonomy at weight 2 anyway).
I've filed an issue with a patch to check empty(taxonomy override selector) here #619726: Ensure normal (non-HS) taxonomy form items get translated via i18n, and that's module's devolpment is fairly active so hopeful that (or a better patch) will get in.
Attached patch moves the weight back to -1 I believe (unless I fail at basic copy, past, edit 2 to -1 ) and unsets the #options at the end of hierarchical_select_process. Tested briefly with forum and content_taxonomy, and both seem to be working.
(another module that could be tested is comment alter taxonomy since they also remove the vocabulary from the node edit form, but I don't use that module).
Comment #35
TripleEmcoder commentedWow, now I got lost in this discussion ;)
I understand that you agreed on a solution involving module weight correction plus some changes in i18n_taxonomy? Could please sum up the current situation and point to the patches that need to be installed to get all this in shape (I counted 2)?
I am using all of the mentioned modules on my site, i.e. the HS suite, forum, content_taxonomy and i18n_taxonomy. Will it work?
Comment #36
hefox commentedTheoretically it'll work, but I'm using neither i18ntaxonomy nor forum, though I tested briefly for forum. I'd love for you to test it since you are using those! XD
1) The patch for i18ntaxonomy is for Upgrade to the dev verison if i18ntaxonomy (or guess where the line would go in current verison; it's fairly simple)
2) If you applied my previous patch, undo it via this sql statement :
3) Upgrade latest dev of hierarhical select.
4) Apply the patch
5) Run update.php for hierarchical select; update 2.
That should be it.
New patch, haven't tested it, but quickly added in the $description filter_xss_admin on vocabulary help.
Comment #37
summit commentedSubscribing, greetings, Martijn
Comment #38
mrfelton commentedpatch at #36 fixes a conflict with the cck_taxonomy_subset module. Thanks.
Comment #39
wim leersThanks mrfelton!
I'd like one more confirmation :)
Comment #40
TripleEmcoder commentedOk, I finally got around to testing this patch. Sorry for taking so long. It does solve the issue and doesn't seem to brake anything (as far a I can tell for now). But the patch for i18n_taxonomy produces a syntax error :D Easy to fix though.
Comment #41
hefox commentedmemfis, could you redo the patch for il8n_taxonomy, or post in the ie8n taxonomy issue the patch is from on what the quick fix is (bump the issue, yay \o/)? I don't have a testing enviroment set up to quickly do it and bit busy atm.
Comment #42
TripleEmcoder commentedOk, so just be sure I captured the intent. What you want to check there is that taxonomy_override_selector is FALSE or not set, right? Can you look below?
Broken:
Fixed:
Is this what you meant? I honestly don't know what this does, just tried to figure out from code a semantically equivalent but syntactically correct statement ;)
Comment #43
Bilmar commentedsubscribing
Comment #44
hefox commentedhttp://drupal.org/node/619726#comment-2373804 Updated i18n taxonomy patch, though manually so hopefully it's fine. It'd be very useful if those who have reviewed it say in that thread that they have reviewed it since right now it's just me in that thread.
Sigh, forgot can't do empty with functions... again.
I did ! instead of == FALSE; is there any advantage to == false over not (!)?
Comment #45
baaj commentedSweet! applied both patches (#36,#44) and it now works
Much appreciated hefox et al
Comment #46
wim leersCan you please combine them into a single patch so it's easier to test and review? Thanks.
Comment #47
TripleEmcoder commented@hefox: they are the same thing, I just used == FALSE because it was easier to read the intent with the default parameter also set to FALSE.
Comment #48
YK85 commentedsubscribing
Comment #49
Alex Andrascu commented+1
Comment #50
wim leers[rant]Wow, just read through the entire issue again (but skipped some parts, otherwise it'd take *way* too long) and this issue further decreases my faith in Drupal's Forms API and Drupal in general. This is a truly ridiculous edge case and should never occur in the first place.[/rant]
I've added my vote of confidence at #619726: Ensure normal (non-HS) taxonomy form items get translated via i18n, which will hopefully help in getting that patch committed sooner rather than later.
I've committed the patch in #36, with some changes. See http://drupal.org/cvs?commit=335120
Thanks for your hard work and persistence, bibo and of course most of all, hefox! :)
Comment #51
klonosWow Wim! You're on fire!!...
I see you've committed a whole lot of patches and code that needs testing. When are we to see a new dev release available? Say 12 hours or so?
Note to self: Need to get some rest and be ready for some testing once a (dev)release is out.
Comment #52
wim leersI think the dev snapshots refresh every 4 hours. You can ensure you've got all patches by comparing the date and time at which the snapshot was generated with the date and time of the last commit.
Comment #53
klonosIt's either #728708: Once you install the latest stable, you aren't offered newer dev releases in site's 'Available updates' page... or I am a complete newbie ;)
Comment #54
wim leersSee my reply :)
In any case, you can just download the dev snapshot from http://drupal.org/project/hierarchical_select!
Or even better: get it directly from CVS!