688405 - rowspan values are capped at 8190

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[steve.bowman]

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.

Comment 1

[steve.bowman]

Attachment: A nested table containing 8196 rows

Comment 2

[:aceman]

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?

Comment 3

[Boris Zbarsky [:bzbarsky]]

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...

Comment 4

[Boris Zbarsky [:bzbarsky]]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, and we'd probably need to coordinate that change with WebKit.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Attachment: jprof profile at 2ms period

~500 hits ~= 1 cpu second.  Loads quickly.  Profile taken under a trunk build from last night

Comment 7

[:aceman]

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).

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 9

[Bernd]

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.

Comment 10

[steve.bowman]

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...

Comment 11

[Boris Zbarsky [:bzbarsky]]

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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Allow rospans up to 65534, up from our current 8190.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Consider the typo in the checkin comment fixed.  ;)

Comment 14

[:aceman]

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?

Comment 15

[Bernd]

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.

Comment 16

[:aceman]

Could there be a wiki page where these arbitrary caps would be listed?

Comment 17

[Boris Zbarsky [:bzbarsky]]

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.

Comment 18

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/919a0a777520

Comment 19

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/919a0a777520

Comment 20

[j.j.]

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

Comment 21

[steve.bowman]

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?

Comment 22

[steve.bowman]

(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?

Comment 23

[j.j.]

(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

Comment 24

[j.j.]

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

Comment 25

[j.j.]

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/

Comment 26

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 27

[steve.bowman]

(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.

Comment 28

[:aceman]

(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.

Comment 29

[j.j.]

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.

Comment 30

[:aceman]

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.

Comment 31

[Eric Shepherd [:sheppy]]

Listed on Firefox 9 for developers; the <td> page already lists the 65534 value as the maximum.