Closed Bug 688405 Opened 10 years ago Closed 10 years ago
rowspan values are capped at 8190
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:6.0.2) Gecko/20100101 Firefox/6.0.2 Build ID: 20110902133214 Steps to reproduce: Published a nested table containing 1 item linked to 15000 child items to Firefox using the Publish Table feature of our Cradle software. Actual results: The table renders correctly in Internet Explorer 8, but publishing the same table to Firefox 6.0.2 after table row 8190, the inner table columns are not rendered correctly and it appears that the inner columns appear to have been shunted left overwriting the outer item columns and the inner columns are blank. Incidentally I tested this in Google Chrome, which I think uses the Safari rendering engine and Chrome has the same problem as Firefox. The only browser that currently renders the table correctly is IE8. Expected results: The nested table should be rendered correctly as per Internet Explorer 8.
Confirming on FF6 and 9, Win XP. Yes, IE8 renders it correctly, but needs about 4 minutes for initial load and to jump to the end of the page (on C2D CPU). What users can use such a page? In which configuration does it work usably for you?
Status: UNCONFIRMED → NEW
Component: General → Layout: Tables
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → layout.tables
Webkit and Gecko both cap rowspan at 8190. colspan is capped at 1000 in all browsers I know of. The 8190 cap is caused by the fact that the data structure used to store the rowspan only has 13 bits to store it in. Not sure why the cap is 8190 and not 8191, though. I believe this is also used to prevent accidentlal DoS issues, so the cap is unlikely to go away...
That said, since colspan is capped to 1000 but also has 13 bits, we could steal 3 bits from colspan for rowspan. That would allow us to have rowspans up to 65535 or so, assuming we want that. Bernd, thoughts?
Oh, and we'd probably need to coordinate that change with WebKit.
Summary: nested table inner columns overwriting outer columns after 8190 rows → rowspan values are capped at 8190
~500 hits ~= 1 cpu second. Loads quickly. Profile taken under a trunk build from last night
Yes, allowing larger rowspan than colspan would make more sense. It would actually fix this report. Randell, what are you measuring? The attached testcase takes about 2seconds to render for me, there doesn't seem to be a problem (compared to those 4 minutes of IE8). Increasing to 65535 wouldn't be too bad (and a bit of render time would be expected).
bz asked for a profile, probably noticed the 4min remark and also was slow loading for him - but he was doing a build at the time. I see no perf problem here. IE8's table code obviously has something like an O(n^2) problem
For me the increase would be fine, I am not sure that the DOS would not hit us more. >Not sure why the cap is 8190 and not 8191, though. Because, I am very used to this number from my work on hardware project, I introduced the number in bug 141818.
Wow! That was fast, thanks to everyone, for their efforts in getting to the bottom of this issue, so quickly. We have a number of customers who are always dealing with enormous amounts of information as you can see by this report, which actually I have cut down as the original was 15000 rows, this stems from a customer request who have a single item linked to over 8000 items and they needed the ability to publish this to a html report and as a result we too have had to remove our hard coded 8000 limit to allow the customer to display more than 8000 linked items, and thats why we then we hit your rowspan limit. In my experience, having lots of customers who are forever pushing these boundaries (NASA), who use Firefox and not IE, I am surprised that colspan has such a high limit of 1000, from all the complex tables we have dealt with over the years, the most complicated tables I have seen have are up to 100's cols and I have never seen a table with over 500 cols, but then I can imagine that there will be some area in science who has exceeded 500 cols? with all the complex tables I have seen 1000's or even 10's of 1000's of rows are more likely. Also I was wondering since the attached report when published hits this rowspan cap on Firefox, Chrome and Safari, which I assume is due to the fact that that all the above browsers are based on Gecko and Webkit? does this mean that if you fix this is Firefox and coordinate the change with Webkit, that the result would have the ripple effect and remove this cap for all the other browsers also? Thanks (In reply to Boris Zbarsky (:bz) from comment #3) > Webkit and Gecko both cap rowspan at 8190. colspan is capped at 1000 in all > browsers I know of. > > The 8190 cap is caused by the fact that the data structure used to store the > rowspan only has 13 bits to store it in. Not sure why the cap is 8190 and > not 8191, though. > > I believe this is also used to prevent accidentlal DoS issues, so the cap is > unlikely to go away...
Steve, Chrome and Safari are based on WebKit. So yes, if we change this in Gecko that will change it in Firefox, and if it gets changed in WebKit that will change the behavior in Chrome and Safari.
Consider the typo in the checkin comment fixed. ;)
Assignee: nobody → bzbarsky
Whiteboard: parity-IE → [need review][parity-IE]
Reporter, could you try what is the limit in IE (making a page with rowspan=100000 or more)? Maybe we could match it. bz, what about issuing a notice into the Error console whenever a page hits this limit (cap). I think whatever the limit ends up, there will always come somebody with a synthetic page that hits it. The notice/warning will make him know it is a design decision in Firefox, not a bug. I think there should be such warnings about any limits FF imposes that are not defined in the HTML spec. What do you think? Would it be too costly?
We introduced the cap in 2005 and it took 6 years to get this cap lifted, I just do not see the point of the warning just for a very few if not a single person in the next 5 years, I would prefer to dismiss the bugs as they pop up as wontfix.
Could there be a wiki page where these arbitrary caps would be listed?
aceman, warning here would be a bit of a pain, since it would need to happen deep inside the guts of attribute-parsing code. It would be a (small) perf hit for every single numeric attribute. Furthermore, if we replace the cellmap with something better (always a possibility I'm thinking about), this restriction is likely to go away. In practice, any such restrictions that are _really_ cross-browser (e.g. the colspan cap) should be in the spec... If they're not there, they should be added.
Whiteboard: [need review][parity-IE] → [parity-IE]
Target Milestone: --- → mozilla9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Steve, feel free to file a bug against WebKit https://bugs.webkit.org/ (In reply to aceman from comment #16) > Could there be a wiki page where these arbitrary caps would be listed? Feel free to document it here: https://developer.mozilla.org/en/HTML/Element/td https://developer.mozilla.org/en/HTML/Element/th
j.j, OK, will do, so presumably I would need to file the same bug with the WebKit Bugzilla system, which I also assume is a totally different site, in which case I will have to register an account with that site also?
(In reply to Boris Zbarsky (:bz) from comment #18) > http://hg.mozilla.org/integration/mozilla-inbound/rev/919a0a777520 Thanks to all involved for fixing this bug so quickly. Boris, I see you have targed the fix for this bug for Target Mozilla 9, the current Firefox version is 6.0.2 and Firefox 6.0.2 is based on Mozilla 5.0, I dont suppose you know yet approximately when Mozilla 9 is scheduled to be released, just so I have some kind of idea? I know that back in June Mozilla announced a 5-week rapid release cycle for Firefox, but this bug was in Mozilla so presumably its Mozilla9 I need to lookout for and not a Firefox release?
(In reply to steve.bowman from comment #21) > which I also assume is a totally different site, in > which case I will have to register an account with that site also? Yes, it's unrelated to Mozilla. However you should refer to our bug here. (In reply to steve.bowman from comment #22) > Boris, I see you > have targed the fix for this bug for Target Mozilla 9, the current Firefox > version is 6.0.2 and Firefox 6.0.2 is based on Mozilla 5.0, Not really. Fx6 is based on Mozilla 6. The "Mozilla 5.0" user agent string is a legacy (meaning Netscape Navigator 5) Fx9 is based on Mozilla 9, release is planned for 2011-12-20 https://wiki.mozilla.org/RapidRelease/Calendar
or, to be more confusing: Fx6 is based on Gecko 6 which is also called Mozilla 6 these days. The "Mozilla 5.0" user agent string is a legacy (meaning Netscape Navigator 5) Fx9 is based on Gecko 9, also called Mozilla 9 https://developer.mozilla.org/en/Gecko
BTW, Firefox 9 is in the "Aurora" channel in one or two days. You can download and test then. http://www.mozilla.org/firefox/channel/
> Steve, feel free to file a bug against WebKit https://bugs.webkit.org/show_bug.cgi?id=68714 is filed. I just forgot to mention it, sorry.
(In reply to Boris Zbarsky (:bz) from comment #26) > > Steve, feel free to file a bug against WebKit > > https://bugs.webkit.org/show_bug.cgi?id=68714 is filed. I just forgot to > mention it, sorry. Thanks, Boris, no problem, that saves me having to set up another account with the Webkit Bugzilla.
(In reply to j.j. from comment #20) > Feel free to document it here: > https://developer.mozilla.org/en/HTML/Element/td > https://developer.mozilla.org/en/HTML/Element/th Done.
I don't know whether it's worth to mention this in https://developer.mozilla.org/en/Firefox_9_for_developers Keeping dev-doc-needed, should others decide.
Well, opera also chokes on the testcase, took 15 minutes on the 8196 rows. But it does not have the problem, it probably has the limit set higher. I just didn't have the guts to test more rows today. I can confirm with todays nightly 9 (20110927030845) that the problem in the testcase is fixed.
Status: RESOLVED → VERIFIED
Listed on Firefox 9 for developers; the <td> page already lists the 65534 value as the maximum.
You need to log in before you can comment on or make changes to this bug.