Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Error when initializing on detached select #358

Closed
DiogoDoreto opened this Issue Nov 21, 2011 · 28 comments

Comments

Projects
None yet

Chosen throws an error when I try to init a select that isn't on DOM yet

Example on Chrome Console:

> $('<select />').chosen()
TypeError: Cannot set property 'disabled' of undefined

ps: I'm using the jQuery version.
Thanks

We were having this problem too.

The set_up_html function is searching the DOM for the container, then basing all other searches (e.g., for the search field) off of that. Since that function is also responsible for creating the container a mere few lines prior, the DOM search appears to be superfluous (though we don't know the plugin well enough to be sure of this).

We were able to patch it for ourselves by changing line 289 of chosen.jquery.js (v0.9.5) from:

this.container = $('#' + this.container_id);

to:

this.container = container_div;

yethee commented Dec 7, 2011

@elliterate, thanks for the patch.

@elliterate thank you! I think this is the right thing to do since it's creating the container_div right above...

maccman commented Feb 21, 2012

+1 - just been bitten by this

kromped commented Mar 13, 2012

+1

Jsuis la dans 2 min

Patrick de Lanauze
patrick.delanauze@gmail.com
On Mar 13, 2012 4:29 PM, "kromped" <
reply@reply.github.com>
wrote:

+1


Reply to this email directly or view it on GitHub:
#358 (comment)

+1

This is impossible because chosen puts itself next to the passed in select. if your select isn't in the dom where would it go? answer: it would go to hell. close this issue please.

@delvarworld

Not sure why you decided to drop in out of nowhere and leave an unhelpful, insulting, and just plain wrong remark, but let me explain why this 'bug' should be considered, and why the suggested fix above works in most if not all scenarios (albeit that assertion is untested). When a node is created using document.createElement(), it exists in memory but is not yet attached to the DOM. The DOM owns it, but cannot access it from the document root using methods such as any of the document.getElementByX() methods, although with a reference to the detached node these methods function normally.

Thus, if you look at the patch suggested by @elliterate, you'll notice that he is essentially switching out assignment via a getElementById (under the jQuery covers) call on the root of the document (which will fail if the node you've called chosen on is in a detached state) with a direct assignment to the detached container node stored in memory.

So why is this not only possible, but useful? Well, detached nodes are used for a great many things. In my case, using Backbone.js, each View owns its own detached element that can be manipulated and populated with whatever, and eventually attached/detached/re-attached to/from the DOM. So, with a reference to the detached node, Chosen can go right ahead inserting elements into and binding events to the detached node, and everything will be peachy.

As I said before, I don't know if the suggested patch is perfect, as I haven't thoroughly tested it, but it's quite obvious that you do not know what you're talking about, and should therefore either refrain from commenting on topics you don't understand, or if you must comment, do so politely.

I was being helpful (btw "+1" is a really helpful remark dude), and you don't understand what I'm saying. The issue is that the select in the original issue has not parent so putting select.after(chosen) will put chosen into nowhere. It doesn't make sense to call chosen on a select that has no parent element.

@delvarworld

The original example is contrived (obviously so as @DiogoDoreto mentions the example is from the Chrome console), but the rest of us were able to see through the trivial incorrectness of the contrived example to the underlying issue (upon which I have elaborated).

I found this article via a Google search when experiencing the same error, and it seemed (and still seems) a valid point/suggestion that should remain open until addressed in one way or another by the authors of the software so that others having the same experience can add their voice to the mix if they so desire. So unless you are the author of this software, have any helpful contributions to provide, or more generally wish to discuss software with your peers in a more congenial manner, just go your own way and don't worry about it.

kromped commented Mar 21, 2012

@delvarworld @Phylodome Think about the programmer using this API. I had to do $('.blah').chosen() after using jquery's clone method because the clone doesn't exist in the DOM yet, which is quite annoying and only works after I have appended it to the DOM. The whole point of this library is that is abstract from the programmer so he doesn't have to deal with it and it should just work.

Think about this use case:

<select id="foo"><option>bar</option></select>

Now you want to clone this because you're dynamically inserting it into the dom. But guess what? You want it to be chosen so when you clone it, it attaches all the events to it, but this bug prevents cloning from working properly and that you to "re-chosen it".

Hi @kromped.

In your example, the node exists in the DOM after a clone, it's just detached. This is the exact situation I was describing in my earlier post.

As for cloning a chosen node, you can use the withDataAndEvents and deepWithDataAndEvents flags on jQuery's clone method, but if you cloned a node on which you'd already initialized Chosen, the events would still remain bound to the original as well, which you wouldn't want. So, given the patch suggested earlier, you can initialize Chosen on a cloned node directly, but you'll still need to initialize once per clone, or initialize all the clones together after you're finished cloning but before you attach to the DOM.

@Phylodome

The absolute best part of this? I already opened this ticket: #132 which was a duplicate of #92

just kidding, the best part is that I already fixed it in my fork months ago but stopped pushing live because chosen pull requests aren't being accepted https://gist.github.com/2153138

so instead of "+1" and trolling a ticket and telling other to do work, I actually fixed it, submitted a pull request, was ignored, and stopped caring. If a project has about 600 lines of code, 5,000 forks, 187 issues and 29 unanswered pull requests, it's not time to say "+1", it's time to fork it and fix it yourself.

close this issue

@delvarworld

None of the tickets you mention are related to this issue.

And I've already patched this in the fork my company is tracking. This is not trolling, nor is it telling others to do work (which, as it happens, is a consequence of creating/maintaining a popular repo); it is simply a fact that this portion of the plugin is sub-optimal and should be patched.

It's funny that you tell me that I'm trolling and trying to get the owners of this repo to do work, when in fact you are the one who showed up here with nothing helpful or useful to say (trolling), asking for this thread to be closed (now help me out, would that be considered work in your book?) without even understanding what was being discussed.

I'm seeing this issue. Please let me know if there's anything I can do to help.

I would also appreciate a fix to this issue.

This is relevant in our case because we render a block of DOM, configure it with e.g. chosen instances, and then attach it to the page. So the chosen is able to configure itself after the select, since the select has a parent element.

I would also love to help with this. It seems pretty simple, and it's 7 months old.

+1

newbyca commented Dec 2, 2012

late to the party, but ... +1

dashk commented Dec 14, 2012

+1

dnlek commented Jan 18, 2013

+1

jimeh commented Jan 31, 2013

+1

emilisto commented Feb 7, 2013

+1

amclin commented Feb 27, 2013

Fix for the latest master supplied in Pull #1061

This issue is line 373 in the current version of chosen.jquery.js

Contributor

pfiller commented Apr 18, 2013

This is a duplicate issue of #183 and will be resolved by #1151. Thanks for the lively discussion.

@pfiller pfiller closed this Apr 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment