Bug 4571

Turns out there was a third test affected: HTMLTableElement39

Actually, rowIndex seems broken wrt to putting <thead> first from looking at the code (it only puts tfoot last). Do you (Darin) want a separate BR + testcase for that?

Yes, lets use a separate bug report for that.

Looks like Maks has a great fix for this in the KDE KHTML tree. We should get that.

I haven't committed it yet, but it's at http://www.cs.cornell.edu/~maksim/patches/table_rows_order.diff (the diff also includes a fix to rowIndex). The patch is only sane because we cache more stuff than you do, though.

Anders is looking into this.

Please don't forget to update the DOM catalog after fixing this (or remind me to).

<rdar://problem/4288297>

Created attachment 16362 [details] fix HTMLCollection07 and HTMLCollection08 This patch fixes the test cases HTMLCollection07 and HTMLCollection08, and also the bug #8133 (maybe it could be marked as duplicated?). I extended HTMLCollection and overrided traverseNextItem() (rather than modifiy HTMLCollection::traverseNextItem(), i think is more readable and more maintenable like this. I copy/pasted the header for the both new files, i guess the copyright are wrong; is there a header sample on the wiki or elsewhere? I ran the tests, and i didnt see new regression.

Comment on attachment 16362 [details] fix HTMLCollection07 and HTMLCollection08 The general concept looks good. However, there are a few issues. 1) I see a lot of code using braces around single-line if or while bodies like the below. In WebKit coding style, we do not use braces in such cases. + if (table->firstTBody()) { + current = table->firstTBody()->rows()->firstItem(); + } 2) In general, the logic seems more complicated than necessary. I think just by adding a flag that says whether you haven't started, are currently doing the thead, are currently doing bodies, or are currently doing the tfoot, you could make the logic much simpler. You could do the thead as the first phase, then in phase two do the normal traversal (which will find trs that sit directly in the table, which can happen in XHTML) but refusing to enter thead or tfoot elements, then finally do the tfoot if there is one. 3) It looks like if there is a tfoot, your code will miss tr elements that sit directly in the table or are otherwise misnested (which can happen with XHTML or via direct DOM operations). However, if there isn't a tfoot but there is a tbody, your code will find them. The general approach is good and it's great that you are doing this fix, but please fix up the coding style issue and consider the correctness issue in #3. It might help to add additional test cases to cover weird edge cases like that.

Comment on attachment 16362 [details] fix HTMLCollection07 and HTMLCollection08 r- for above comments. Please address.

*** Bug 8133 has been marked as a duplicate of this bug. ***

HTML5 makes this explicit, fwiw.

According to the log of r29101, HTMLCollection07 and HTMLCollection pass. So the bug is still valid? (r29101 | darin@apple.com | 2008-01-03)

Fixed in r29101.