Bug 12124 - RenderTable::section{Above,Below}() can return the wrong answer for the {footer,header}
Summary: RenderTable::section{Above,Below}() can return the wrong answer for the {foot...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-05 07:58 PST by mitz
Modified: 2007-01-11 20:46 PST (History)
0 users

See Also:


Attachments
Patch (42.48 KB, patch)
2007-01-06 07:38 PST, mitz
no flags Details | Formatted Diff | Diff
Patch (42.47 KB, patch)
2007-01-07 00:49 PST, mitz
no flags Details | Formatted Diff | Diff
Patch (42.46 KB, patch)
2007-01-08 11:31 PST, mitz
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2007-01-05 07:58:47 PST
RenderTable::sectionAbove() and sectionBelow() skip the last/first child when you ask for the section adjacent to the footer/header. (I ran into this problem while working on another patch and I don't want to include the fix in that patch since it involves updating the results of many Mozilla table tests).
Comment 1 mitz 2007-01-06 07:38:49 PST
Created attachment 12257 [details]
Patch
Comment 2 Darin Adler 2007-01-06 17:09:44 PST
Comment on attachment 12257 [details]
Patch

I think resetSectionPointerIfNotBefore needs an assertion that o is a table section as the caption code does that o is a RenderBlock; in fact, I'm not at all sure that there's a guarantee that o is a RenderBlock.

Otherwise r=me.
Comment 3 mitz 2007-01-07 00:49:37 PST
Created attachment 12273 [details]
Patch

(In reply to comment #2)
> (From update of attachment 12257 [details] [edit])
> I think resetSectionPointerIfNotBefore needs an assertion that o is a table
> section as the caption code does that o is a RenderBlock; in fact, I'm not at
> all sure that there's a guarantee that o is a RenderBlock.

The assignments of o (to ptr and to m_caption) where either no-ops or 0 assignments. I changed things to not do the no-op and to make the 0 assignment explicit, so no cast is needed.
Comment 4 Darin Adler 2007-01-08 10:34:13 PST
Comment on attachment 12273 [details]
Patch

+                        m_firstBody = static_cast<RenderTableSection*>(child);

Looks like this is indented too far.

Looks great to me. r=me, but needs review by Hyatt too.
Comment 5 mitz 2007-01-08 11:31:29 PST
Created attachment 12304 [details]
Patch

Fixed indentation.
Comment 6 Dave Hyatt 2007-01-11 20:33:57 PST
Comment on attachment 12304 [details]
Patch

r=me
Comment 7 Mark Rowe (bdash) 2007-01-11 20:46:49 PST
Landed in r18791.