Closed
Bug 99923
Opened 24 years ago
Closed 24 years ago
Table.cellPadding does not work
Categories
(Core :: Layout: Tables, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bernd_mozilla, Assigned: bernd_mozilla)
References
Details
(Keywords: testcase, Whiteboard: PATCH CANDIDATE_094)
Attachments
(5 files)
694 bytes,
text/html
|
Details | |
692 bytes,
text/html
|
Details | |
692 bytes,
text/html
|
Details | |
595 bytes,
text/html
|
Details | |
860 bytes,
patch
|
karnaze
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
see attached testcase, needless to say that a competitors product renders the
testcase fine, moz 2001-09-10-03 win98
Assignee: jst → karnaze
Component: DOM HTML → HTMLTables
QA Contact: stummala → amar
Summary: Table.cellPadding does not work → Table.cellPadding does generate a correct reflow
Updated•24 years ago
|
QA Contact: amar → stummala
Summary: Table.cellPadding does generate a correct reflow → Table.cellPadding does not work
![]() |
||
Comment 7•24 years ago
|
||
OK. Here's what's up. nsHTMLTableElement.cpp uses NS_IMPL_STRING_ATTR to
implement the GetCellPadding and SetCellPadding functions. This lives in
nsGenericHTMLElement.h and just updates the attribute. It does not send the
document a change notification, so there is no reflow.
I tried hacking that macro to include a call to AttributeChanged() but that did
not seem to help. I could well be passing the wrong args to AttributeChanged;
I'll play with it more.
Boris, I am afraid that you are wrong, the code looks rather symmetric for
cellPadding and cellSpacing, cellSpacing generates a reflow, cellPadding does not.
Comment 9•24 years ago
|
||
Boris, the table code should be notified when one of its attribute changes, and
I think that's happening properly. Like Bernd pointed out, the DOM code is the
same for cellspacing and that one works fine.
![]() |
||
Comment 10•24 years ago
|
||
Yeah.... I found that this morning when I went back to look at it.
Serves me right for debugging at night. :)
Comment 11•24 years ago
|
||
adding amar in cc list and changing the qa contact.
QA Contact: stummala → amar
Assignee | ||
Comment 12•24 years ago
|
||
at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableRowFrame.cpp#843
we skip the necessary reflow for the cell
the following patch fixes the problem but I have no clue about the performance
impact
Index: nsTableRowFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/table/src/nsTableRowFrame.cpp,v
retrieving revision 3.274
diff -u -w -r3.274 nsTableRowFrame.cpp
--- nsTableRowFrame.cpp 2001/08/06 14:04:58 3.274
+++ nsTableRowFrame.cpp 2001/09/21 19:04:51
@@ -838,6 +838,7 @@
if ((availWidth != cellFrame->GetPriorAvailWidth()) ||
(cellDesiredSize.width > cellFrame->GetPriorAvailWidth()) ||
(eReflowReason_StyleChange == aReflowState.reason) ||
+ (eReflowReason_Resize == aReflowState.reason) ||
isPaginated) {
// Reflow the cell to fit the available width, height
nsSize kidAvailSize(availWidth, aReflowState.availableHeight);
turning on the DEBUG_TABLE_REFLOW in makefile.win in layout\html\table\src give
the following :
Tbl 00BF94D0 r=0 a=8940,UC c=0,0 cnt=0
Tbl 00BF96DC r=0 a=8940,UC c=4440,UC cnt=1
RowG 00BF989C r=0 a=UC,UC c=UC,UC cnt=2
Row 00BF9A1C r=0 a=UC,UC c=UC,UC cnt=3
Cell 00BF9BE4 r=0 a=UC,UC c=UC,UC cnt=4
Block 00BF9C40 r=0 a=UC,UC c=UC,UC cnt=5
Block 00BF9C40 d=300,285 me=300
Cell 00BF9BE4 d=360,345 me=360
Row 00BF9A1C d=UC,345
RowG 00BF989C d=UC,345
Initialized min=450 des=450 pref=450
Balanced min=450 des=4470 pref=450 cols=4380
RowG 00BF989C r=2 a=4440,UC c=4440,UC cnt=6
Row 00BF9A1C r=2 a=4440,UC c=4440,UC cnt=7
Cell 00BF9BE4 r=2 a=4380,UC c=4320,UC cnt=8
Block 00BF9C40 r=2 a=4320,UC c=4320,UC cnt=9
Block 00BF9C40 d=4320,285
Cell 00BF9BE4 d=4380,345
Row 00BF9A1C d=4440,345
RowG 00BF989C d=4440,345
Tbl 00BF96DC d=4470,435
Tbl 00BF94D0 d=4470,435
Tbl 00BF94D0 r=1 a=8940,UC c=0,0 cnt=10
Tbl 00BF96DC r=1 a=8940,UC c=4440,UC cnt=11
Initialized min=450 des=450 pref=450
Balanced min=450 des=4470 pref=450 cols=4380
RowG 00BF989C r=2 a=4440,UC c=4440,UC cnt=12
Row 00BF9A1C r=2 a=4440,UC c=4440,UC cnt=13
Row 00BF9A1C d=4440,345
RowG 00BF989C d=4440,345
Tbl 00BF96DC d=4470,435
Tbl 00BF94D0 d=4470,435
Note the missing cell and block reflow in the last reflow.
Assignee | ||
Comment 13•24 years ago
|
||
Chris, I think I nailed down the issue, as my workload is now high, if you would
like to see this in the tree you have to take this bug. Otherwise it has to wait
for another 2 weeks.
Assignee | ||
Comment 14•24 years ago
|
||
The patch improves the picture on bug 54382, while it is not a complete fix.
Blocks: 54382
Assignee | ||
Comment 16•24 years ago
|
||
Comment 17•24 years ago
|
||
Comment on attachment 57792 [details] [diff] [review]
patch
r=karnaze
Attachment #57792 -
Flags: review+
Comment 18•24 years ago
|
||
Comment on attachment 57792 [details] [diff] [review]
patch
sr=attinasi | I have to say, it is ever more confusing that a StyleChange
reflow and an incremental reflow with reflowCommand type StyleChanged are
different...
Attachment #57792 -
Flags: superreview+
Assignee | ||
Comment 19•24 years ago
|
||
>I have to say, it is ever more confusing that a StyleChange
>reflow and an incremental reflow with reflowCommand type StyleChanged are
>different...
the difference is, who is the target of the style change reflow, all frames
higher in the hierarchy pass the incr_reflow with type style change trough, and
the target accepts the reflow and notifies its childrens of the style change
reflow. Clear as mud, for me at least ( it took me two weeks to come this
enlighted point)
Assignee | ||
Comment 20•24 years ago
|
||
patch checked in, testcase also, adding some status foo, if this should go on
any branch the fix for bug 108340 is also necessary.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Whiteboard: PATCH CANDIDATE_094
You need to log in
before you can comment on or make changes to this bug.
Description
•