Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dropdown issue "too much recursion" #6835

Closed
Yohn opened this issue Feb 8, 2013 · 24 comments
Closed

dropdown issue "too much recursion" #6835

Yohn opened this issue Feb 8, 2013 · 24 comments
Labels
Milestone

Comments

@Yohn
Copy link
Contributor

Yohn commented Feb 8, 2013

I think when we took out support for the touchstart feature in the dropdowm menus, we missed something..
at the bottom of the dropdown plugin theres this..

$(document)
  .on('.dropdown-menu', function (e) { e.stopPropagation() })

shouldnt this line have an action in it? like click.dropdown-menu instead of just .dropdown-menu? whats this line for anyways?

I first found this problem a yesterday when I was getting a "too much recursion" error when working with @jschr's bootstrap-modal plugin and reported it there cause it didnt happen without jschrs plugin

my work arounds would be to either add the click event handler in that line, or simply take it out, both solutions work but I'm not sure exactly what its doing so I wouldnt take it out just yet..

guess I should show ya a fiddle to - http://jsfiddle.net/mh4hY/
fiddle works now since jschr updated his plugin with the code posted below

@andriijas
Copy link
Contributor

Got it as well

@andriijas
Copy link
Contributor

A bit trigger happy on the 2.3 release perhaps? :)

@jschr
Copy link

jschr commented Feb 8, 2013

Changing $(document).off('.modal') to $(document).off('click.modal') fixed my plugin.

Still don't quite understand why that caused an error so any insight would be appreciated.

@tinogo
Copy link

tinogo commented Feb 9, 2013

Jep, adding "click" to the dropdown-menu event handler fixed my issue with fancyBox!

@socoastal
Copy link

I don't want to seem like a complete idiot - and it's probably something I'm doing incorrect on my end, but I at least wanted to throw it out there:

After I updated to 2.3 my regular dropdown menus in navigation no longer work. There was no change in code to the navigation, I just simply updated to 2.3 css/js.

@Yohn
Copy link
Contributor Author

Yohn commented Feb 10, 2013

@jurich they seem to be working fine in the docs so I believe its a problem on your end. check out this fiddle.. http://jsfiddle.net/9WyH8/

@socoastal
Copy link

Thanks for looking into it. Definitely something on my end. Much
appreciated.

On Sun, Feb 10, 2013 at 12:41 AM, Yohn [email protected] wrote:

@jurich https://github.com/JUrich they seem to be working fine in the
docs so I believe its a problem on your end. check out this fiddle..
http://jsfiddle.net/9WyH8/


Reply to this email directly or view it on GitHubhttps://github.com//issues/6835#issuecomment-13344787..

Justin Urich

@andriijas
Copy link
Contributor

Its not in his end. There is a bug causing to much recursion with jquery
1.8.x in bootstrap-dropdown.js

On Sunday, February 10, 2013, JUrich wrote:

Thanks for looking into it. Definitely something on my end. Much
appreciated.

On Sun, Feb 10, 2013 at 12:41 AM, Yohn <[email protected]<javascript:_e({}, 'cvml', '[email protected]');>>
wrote:

@jurich https://github.com/JUrich they seem to be working fine in the
docs so I believe its a problem on your end. check out this fiddle..
http://jsfiddle.net/9WyH8/


Reply to this email directly or view it on GitHub<
https://github.com/twitter/bootstrap/issues/6835#issuecomment-13344787>..

Justin Urich


Reply to this email directly or view it on GitHubhttps://github.com//issues/6835#issuecomment-13344846..

@it-can
Copy link

it-can commented Feb 10, 2013

I also experience problems with the dropdown... :-(

@artscoop
Copy link

Same problem with the dropdown component and fancybox. I wonder how.

@gordey4doronin
Copy link

I have the same problem with the following row:

.on('.dropdown-menu', function (e) { e.stopPropagation() })

It caused "Stack overflow" error.

Just decided to temporarily remove this row, like @rzhw proposed in his commit
rzhw@523accf

Or to replace with following:

.on('click.dropdown.data-api', '.dropdown-menu', function (e) { e.stopPropagation() })

@nbauernfeind
Copy link

+1 on this. I'm getting the maximum stack recursion issue too in the angular-strap project.

@gustavohenke
Copy link

I'm yet another one with the same problem.

@mgcrea
Copy link

mgcrea commented Feb 22, 2013

👍 please ship 2.3.1 asap!

@andriijas
Copy link
Contributor

+1

On Friday, February 22, 2013, Olivier Louvignes wrote:

[image: 👍] please ship 2.3.1 asap!


Reply to this email directly or view it on GitHubhttps://github.com//issues/6835#issuecomment-13936865.

@elado
Copy link

elado commented Feb 22, 2013

+1

@titovanton
Copy link

yeah-yeah, everyone already know about this bug...

@christoph-hautzinger
Copy link

+1 for 2.3.1 :-)

@richardrails
Copy link

+1

@richardrails
Copy link

so this is answer? This is bad solution. Many people use Fancy and bootstrap.

@andriijas
Copy link
Contributor

This is not a fancybox issue, Im not using it but yet get the recursion error.

Its because of .on('.dropdown-menu', function (e) { e.stopPropagation() }) line in dropdown js which just tries to listen to a non specified event with the .dropdown-menu namespace.

The issue was introduced with #6488 by @nanek - maybe he can explain the reasoning since he is appearently working with some kind of touch behavior library.

@rzhw
Copy link

rzhw commented Feb 26, 2013

@andriijas I don't believe it was intentional; I've attributed it to the first param being removed from line 161, but not the whole line (#6970). To expand on that, that's because .dropdown-menu is used only as a CSS class, not an event namespace.

@fat
Copy link
Member

fat commented Mar 1, 2013

fixed in 2.3.1-wip

@fat fat closed this as completed Mar 1, 2013
@fat
Copy link
Member

fat commented Mar 1, 2013

thanks for reporting!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.