Closed Bug 365173 Opened 14 years ago Closed 14 years ago

incorrectly placed PR_MAX in pref width distribution of colspanning cells


(Core :: Layout: Tables, defect)

Not set





(Reporter: dbaron, Assigned: dbaron)




(3 files, 1 obsolete file)

There's an incorrectly placed PR_MAX in the code for pref width distribution of colspanning cells.  The old code didn't really make much sense.  I found this while writing testcases related to bug 363144 and bug 363874.
Flags: in-testsuite?
Attached patch patch (obsolete) — Splinter Review
I'm not sure why I thought the old logic here was correct.  Since the span-pref and span-min widths on the columns are accumulated as *additions* to the min and pref (since it's easier to divide them up among the columns that way), I should have the PR_MAX outside the additions.  This is compatible with WinIE7 -- not sure why I thought the other way was -- or if I'd considered it.
Attachment #249801 - Flags: superreview?(bzbarsky)
Attachment #249801 - Flags: review?(bernd_mozilla)
Note that the above patch is a diff against a tree to which the patch for bug 363329 has been applied.  That's why it's not a cvs diff.
Comment on attachment 249801 [details] [diff] [review]

+#ifdef DEBUG_dbaron_off
+            printf("table %p col %d span: min=%d pref=%d spec=%d pct=%f\n",
+                   mTableFrame, col, colFrame->GetMinCoord(),
+                   colFrame->GetPrefCoord(), colFrame->GetHasSpecifiedCoord(),
+                   colFrame->GetPrefPercent());

Please move this code to the place where you deleted it first:
and remove the DEBUG_dbaron_off there.

I just forgot in my previous patch the following lines 

+  printf("\nm:%d c:%d p:%d sm:%d sc:%d sp:%d f:%d", GetMinCoord(), GetPrefCoord(),
+         GetPrefPercent(), GetSpanMinCoord(), GetSpanPrefCoord(),
+         GetSpanPrefPercent(), GetFinalWidth());
   printf("\n%s**END COL DUMP** ", indent);

with that change r+
Attachment #249801 - Flags: review?(bernd_mozilla) → review+
Comment on attachment 249801 [details] [diff] [review]

Makes sense.  sr=bzbarsky.  Sorry for the lag...  :(
Attachment #249801 - Flags: superreview?(bzbarsky) → superreview+
I really want this simple debugging code -- the other stuff spews out tons of information I don't need and is much harder to use.
(Or, at least, it's unnecessarily verbose and spread over too many lines.)
Attached patch patchSplinter Review
Attachment #249801 - Attachment is obsolete: true
Fix checked in to trunk, 2007-01-12 17:08 PST (-0800, America/Los_Angeles).
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.