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

Chrome: "Uncaught RangeError: Maximum call stack size exceeded" #70

Closed
streetLAMP opened this issue May 11, 2012 · 20 comments
Closed

Chrome: "Uncaught RangeError: Maximum call stack size exceeded" #70

streetLAMP opened this issue May 11, 2012 · 20 comments
Labels

Comments

@streetLAMP
Copy link

When using the tablesorter with the page sorter (and columns and zebra widgets), I've found that the Chrome browser chokes much, much sooner than FF or IE or Safari browsers. I'm testing a table with 460 rows and approximately 180 columns. FF/IE/Safari are all able to render this in your fork of tablesorter just fine (requires 8 seconds total load time); however Chrome throws the following error message: "Uncaught RangeError: Maximum call stack size exceeded". Maybe an issue of recursion with verifying that all column cells exist and have data? For what its worth, I have all TH cells using the specific parser information (ie class="sorter-digit"). Have you seen this before? Can you fix this issue? If you like, I can post an example on jsfiddle. Thanks, in advance, for your thoughts!

@Mottie
Copy link
Owner

Mottie commented May 11, 2012

I haven't run into this problem before, if you want, you can just email me the file (gmail account, user name wowmotty) or post a jsFiddle, but yes I'd like to see a demo of the issue. Thanks!

@Mottie
Copy link
Owner

Mottie commented May 11, 2012

I'm still not sure about this error, but I've updated the plugin to version 2.3.2 and I did notice that the zebra and columns widget were using an incorrect selector. Hopefully it is fixed now.

@Mottie
Copy link
Owner

Mottie commented May 15, 2012

Any word on if this latest fix still has this error?

@streetLAMP
Copy link
Author

I tested today using your newest files and the recent fixes do not have any effect (positive or negative) on this issue. Just sent you an email with two demo files (one works, one does not) and the specific error message. Looking forward to your hearing what you think. Thanks!

@Mottie
Copy link
Owner

Mottie commented May 16, 2012

Ok I think I found the problem... it's the zebra and column widgets!

So for now, don't use the widgets and I'll try to optimize them, Maybe css selectors would be an even better idea.

Honestly, I think it's a bit overboard to have a table with 240 columns and 260+ rows...

@streetLAMP
Copy link
Author

I'm super-happy you found the problem! I can omit the widgets, but would really love to be able to re-include them again soon, if you think a fix might be possible. CSS selectors might hold promise. Let me know if you want help testing or kicking ideas around.
(My clients have lots of data; big tables, albeit cumbersome, hold the value of enabling relatively quick glances (and sorting) of data to keep tabs on what's going on, as opposed to other more serious data reporting options and statistical tools; so I'm stuck with big giant tables.)

@Mottie
Copy link
Owner

Mottie commented May 19, 2012

Ok this should be fixed in the latest version (v2.3.3)... I tested it on the table you sent me, but with 270 rows and 260 columns with no error. Please double check it for me :)

@streetLAMP
Copy link
Author

Using Chrome, I'm still seeing the same error, with the same hanging result. I've tried it with version 2.3.4, both locally and also pulling from your files online. I see the error only in Chrome (Chrome says it "is up to date" with version 19.0.1084.46 m) on a PC Win7, 64bit OS (but Chrome is running the default version, most likely 32 bit). I see the error in testing the 264 sample I sent you, but it works fine using the 250 row sample. I cleared cache, repeated, w/o any change in effect.

If you're able to put up another update, I'll be sure to get a same day test response back to you. (my weekends are less predictable).

Here's the error message, from Chrome's Console:
Uncaught RangeError: Maximum call stack size exceeded
s
c.querySelectorAll.m
f.fn.extend.find
b.tablesorter.addWidget.format
applyWidget
$.extend.tablesorter.construct
e.extend.each
e.fn.e.each
$.extend.tablesorter.construct
(anonymous function)
f.Callbacks.o
f.Callbacks.p.fireWith
e.extend.ready
c.addEventListener.B

@Mottie
Copy link
Owner

Mottie commented May 21, 2012

Hmm, ok I got it working by removing this css:

    TABLE.displayNone, .displayNone
    {
        display: none;
    }

I guess I need to add code to show the table before processing...

@streetLAMP
Copy link
Author

Okay, when I remove the "displayNone" class, I am able to replicate the 264 row table as working in Chrome. Looking at the console though still shows the "Uncaught RangeError: Maximum call stack size exceeded" message.

Being able to hide the table upon page load, then build it within the pagesorter, and only then display it for the very first time enabled the page display time to be 2x-3x faster (for a table with, err, like 6,000 rows, which I have) as compared to first rendering the entire 6,000 row table and then applying the jQuery pagesorter widget and re-rendering it. Browsers don't like 6,000 row tables, and the pagesorter offers a nifty way of feeding a manageable view of the table to the browser.

Uncaught RangeError: Maximum call stack size exceeded
s
c.querySelectorAll.m
f.fn.extend.find
b.tablesorter.addWidget.format
applyWidget
$.extend.tablesorter.construct
e.extend.each
e.fn.e.each
$.extend.tablesorter.construct
(anonymous function)
f.Callbacks.o
f.Callbacks.p.fireWith
e.extend.ready
c.addEventListener.B

@Mottie
Copy link
Owner

Mottie commented May 21, 2012

The problem now is the columns widget.... I'll work on optimizing it to prevent that error.

@streetLAMP
Copy link
Author

Sweet! That sounds like progress!

@streetLAMP
Copy link
Author

What are your thoughts regarding eta of a fix for this issue?

@Mottie
Copy link
Owner

Mottie commented May 24, 2012

Well, I had it all fixed... then I tested it in IE7.... maybe this weekend?

@streetLAMP
Copy link
Author

Ok - thanks. Appreciate your work, and the update on potential timing. May the force be with you.

@Mottie
Copy link
Owner

Mottie commented May 28, 2012

Ok, this issue should now be fixed in the latest version.... I tested it with 301 rows and 240 columns (72k table cells is just insane!)

I've also found that hiding the tbody during DOM manipulation, just like you were doing, was a bit faster so I've incorporated that into the plugin. Thanks! ;)

The zebra widget should now be much faster and the column widget should work without errors, but I'd still recommend not using the column widget on a huge table without the pager plugin because it needs to clear class names from every table cell, which is extremely slow in IE, before applying new class names. I may work on having the widget remember the last sort and only remove class names from those columns, but stuff like that just never seems to work perfectly.

But, since you were using the pager plugin, I added a new option named initWidgets which when set to false will not run any widget format code. The widget init code is still run, if it exists. The reason is that normally after the table initializes, widgets get applied, then after the pager plugin initializes, all of the widgets will be applied again. So, in your case, the zebra and columns widget will now only be applied to 25 rows instead of 460 rows, making the overall initialization time much shorter!

Lastly, I've reverted the parsing method back to how it was originally done in v2.0.5 because I found it to be much faster, especially in IE (2x faster).

@streetLAMP
Copy link
Author

Holy Cow! Version 2.3.5 is fast, nimble, and throws no JS errors (tested in Chrome, FF). It elegantly works in Chrome and FF with 1,042 rows (1042 rows * 240 cols = 1/4 million cells). Insanity.

I like the initWidgets:false option. That's super cool for big tables.

CSS Typo: There's an extra space in your new CSS class, that is breaking the new 'display:none' functionality. It should read: "table.tablesorter.tablesorter-hidden {..."

Idea: Might be nice to automatically remove the 'tablesorter-hidden' class from within the core packed .js file. Otherwise, this must be done explicitly, which is also fine (but note it within your documentation). For example:
// Show table (do this after applying .tablesorter, but before applying .tablesorterPager(pagerOptions); applying after the pagerOptions will cause zebra stripes to drop out
.removeClass('tablesorter-hidden')
.tablesorterPager(pagerOptions)

Github: while testing, I initially pulled from your github codebase, which seemed to still be at 2.3.4; your download correctly reflects your new v 2.3.5.

I'll send you a test file, in case that helps clarify the need for the explicit ".removeClass('tablesorter-hidden')"

I'll do a bit more bench testing on other browsers.

@Mottie
Copy link
Owner

Mottie commented May 29, 2012

Great! I'm glad it's working for you so far!

Actually the .tablesorter-hidden class is only applied to the tbody and not the table, so the space should be there.

I'm not sure what you mean by needing to remove the class from the packed version... maybe you wanted an option added that names the class?

Yeah, I forgot to include the initWidget docs... they will be included in the next update.... which will be soon.

@streetLAMP
Copy link
Author

Maybe I had too many test pages open... testing again now... more in a few minutes...

@streetLAMP
Copy link
Author

Hah - you are 100% correct. Sorry for the confusion. v 2.3.5 is working fabulously, and needs no modifications. (Applying .tablesorter-hidden to the TBODY, instead of the TABLE fixed all issues. doh.)

Fabulous Work!

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

No branches or pull requests

2 participants