Closed
Bug 352461
Opened 18 years ago
Closed 18 years ago
[FIX]nsCellMap::GetNextCellBelow
Categories
(Core :: Layout: Tables, defect)
Core
Layout: Tables
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bernd_mozilla, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
8.51 KB,
patch
|
bernd_mozilla
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
This is a splitoff from bug 352367 We need a fast lookup function that returns the next real cell below a given cell it should cache as much info as necessary to get quick. We use this inside BasicTableLayoutStrategy. It should take one additional argument whether the cell should be colspan = 1 or colspan > 1. Unluckily the current mechanism calls CellData* data = GetDataAt(aMap, aRowX, aColX, PR_TRUE); which makes this bug dependent on bug 351942.
Assignee | ||
Comment 1•18 years ago
|
||
Bernd, are you planning to do this now that bug 351942 is done?
Assignee | ||
Comment 2•18 years ago
|
||
So I started looking at this. First of all, I can't quite figure out the song and dance with GetEffectiveColSpan. Is that documented anywhere?
http://www.mozilla.org/newlayout/doc/table-layout.html section Effective columns http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableCellFrame.h#191 http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableFrame.h#437 and a couple of AIM sessions from 2001 ;-) Basically if you have at the end of table row a cell which colspan spans beyond any cell in the table then only the colspan that covers the last column with a originating cell is the effective colspan. <table> <tr><td>foo</td><td>bar</td> <td colspan="42">baz</td>/tr> </table> so the colspan is 42, but the effective is 2. All the other 40 go to waste, no cellspacing for them, no width distribution to them etc.
Assignee | ||
Comment 4•18 years ago
|
||
I'll work on this once bug 363329 lands -- it'll be easier with that patch, since we'll have pretty simple needs for GetNextCellBelow then.
Depends on: 363329
Assignee | ||
Comment 5•18 years ago
|
||
Note to self: I think I actually want to do this with a new class (nsCellMapColumnIterator or something).
Assignee | ||
Comment 6•18 years ago
|
||
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #251492 -
Flags: review?(bernd_mozilla)
Assignee | ||
Updated•18 years ago
|
OS: Windows XP → All
Hardware: PC → All
Summary: nsCellMap::GetNextCellBelow → [FIX]nsCellMap::GetNextCellBelow
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 251492 [details] [diff] [review] Proposed patch + // Fast-path for the case when we don't have anything left in the column and + // we know it. + if (mFoundCells == mOrigCells) { + *aRow = 0; + *aColSpan = 1; + return nsnull; + } makes me shiver, from my experience one can rely on "if (mOrigCells)" but I am not so confident that it will always report the correct value otherwise. So it there is some risk with it, however we can probably go with it and fix the bugs in the cellmap later.
Attachment #251492 -
Flags: review?(bernd_mozilla) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #251492 -
Flags: superreview?(roc)
Comment on attachment 251492 [details] [diff] [review] Proposed patch + if (aIncrement == 0) { + // This will span to the end of the row group Document this behaviour in the header?
Attachment #251492 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 9•18 years ago
|
||
Fixed, with that comment added.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•