365173 - incorrectly placed PR_MAX in pref width distribution of colspanning cells

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: reference rendering for testcase

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

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.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on 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 5

[Bernd]

Comment on attachment 249801 [details] [diff] [review]
patch

+
+#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());
+#endif

Please move this code to the place where you deleted it first:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF&root=/cvsroot&file=nsTableColFrame.cpp&rev1=3.90&rev2=3.91#7
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+

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 249801 [details] [diff] [review]
patch

Makes sense.  sr=bzbarsky.  Sorry for the lag...  :(

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Or, at least, it's unnecessarily verbose and spread over too many lines.)

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2007-01-12 17:08 PST (-0800, America/Los_Angeles).