If you go to http://tvtropes.org/pmwiki/pmwiki.php/Main/WikiSandbox and click "Edit", you'll get the assertion: ###!!! ASSERTION: should not be zero if we haven't allocated all pref percent: '(!spanHasNonPctPref || nonPctTotalPrefWidth != 0) && nonPctColCount != 0', file ../../../layout/tables/BasicTableLayoutStrategy.cpp, line 585 You'll also get the assertion when you click on the "Send" button.
Created attachment 298037 [details] gdb stack trace Huh, uploading an attachment during bug creation doesn't seem to work.
I can reproduce on Mac. I'm guessing this is another regression from bug 368504.
I also see this assertion when loading the URL http://en.wikipedia.org/wiki/Chronic_fatigue_syndrome in a trunk checkout from today.
I also see this assertion on attachment 303417 [details] from bug 413286. Specifically, the tables under the headings "Spanning cell has [low|high] percent width".
Created attachment 304312 [details] [diff] [review] fix w/ crashtest Here's a fix, with the reduced testcase attached as a crashtest. PATCH SUMMARY: When we've distributed all the colspan's percent-width, return immediately, rather than continuing to loop across trailing zero-width columns. EXPLANATION: Basically, the issue was with situation with: - we're distributing a colspan's %-width proportional to the spanned columns' pref width - there are empty (zero-pref-width) columns at the end When we're looping and we get to those empty columns at the end, we'd fail the assertion, because nonPctTotalPrefWidth would be zero, since the only columns left are empty. This patch fixes the reduced testcase and all URLs that have been mentioned as triggering this assertion.
FWIW: It might not be immediately obvious why we hit this assertion on the reduced testcase, since it looks like all of its cells have zero width, so we shouldn't be dealing with nonPctTotalPrefWidth in the first place. BUT, the reason we hit the assertion is this: The first column actually has a small nonzero pref width, because of the table's cellpadding/cellspacing. The second column, however, has prefWidth == 0, since no cell is explicitly created there. So we've got a trailing empty column after a nonempty column, which makes us fail the assertion as described in comment 6.
Comment on attachment 304312 [details] [diff] [review] fix w/ crashtest (please nominate for approval only after you get your review)
Comment on attachment 304312 [details] [diff] [review] fix w/ crashtest Why not just fix the assertion to allow for "|| aSpanPrefPct == 0.0f"? Also, while you're there, allocatedPct could be moved inside the if, and not initialized.
Comment on attachment 304312 [details] [diff] [review] fix w/ crashtest Er, never mind. nonPctTotalPrefWidth or nonPctColCount could also be zero at this point. So yeah, this patch is correct. You might actually want to phrase the assertion as a ?:, though. And you should probably move allocatedPct into the if and not initialize it. So r+sr=dbaron with that.
Created attachment 304892 [details] [diff] [review] fix v2 (addressed review comments) > You might actually want to phrase the assertion as a ?:, though. Yup, that makes much more sense. > And you should probably move allocatedPct into the if and not initialize it. Done.
Justification for requesting approval on patch: - Avoids divide-by-zero - Fixes assertion failure
Comment on attachment 304892 [details] [diff] [review] fix v2 (addressed review comments) a=beltzner for 1.9
fix v2 checked in. Checking in BasicTableLayoutStrategy.cpp; /cvsroot/mozilla/layout/tables/BasicTableLayoutStrategy.cpp,v <-- BasicTableLayoutStrategy.cpp new revision: 3.267; previous revision: 3.266 done RCS file: /cvsroot/mozilla/layout/tables/crashtests/413180-1.html,v done Checking in crashtests/413180-1.html; /cvsroot/mozilla/layout/tables/crashtests/413180-1.html,v <-- 413180-1.html initial revision: 1.1 done Checking in crashtests/crashtests.list; /cvsroot/mozilla/layout/tables/crashtests/crashtests.list,v <-- crashtests.list new revision: 1.34; previous revision: 1.33 done