363150 - [FIX]Colspanning cell containing div with specified width makes table too wide

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See testcase gavin posted in bug 363146 comment 4.  Patch coming up.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Pretty self-explanatory.  Alternately, we could set the pref coord before the min coord...

Comment 2

[Boris Zbarsky [:bzbarsky]]

I'm no longer sure this is the right fix, by the way; I'll need to think about it a little bit more and make sure I understand the logic here.

Comment 3

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

Oh, no, I think this is the right fix, although the easier solution is just to switch the two lines.

That said, maybe a better fix is to remove the code I added that made AddMinCoord affect the pref coord and do it manually in the one place that needs it, which I think is actually right here.

Comment 4

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

Er, actually, this was not the reason I added that code to nsTableColFrame.h.  I was thinking of the PR_MAX that's right in the middle of the patch.  (It's a slightly confusing space-optimization, perhaps -- AddPrefCoord really tracks two different numbers, but only stores one since only one ever matters.)

Comment 5

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

And I realize I didn't comment the Span* variables very well -- I'll write a comment explaining what those actually do.

(The basic idea is that column-spanning cells distribute the *extra* space that they need to take up over the columns that they span, so the Span* variables include only the extra amount in adddition to what's done by non-spanning cells.  I think IE uses a 1-pass algorithm that only considers extra relative to non-spanning cells in earlier rows, though, but I don't like that since it's not row-order-invariant, and IE is the only browser that's not row-order-invariant.)

Comment 6

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

Attachment: patch with comments

The patch looks fine; here's a version with additional comments.  At some point I should probably revisit what I wrote in an XXX comment (accumulating the specified coord as a width rather than as a boolean), which I think may make things a bit simpler.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 248013 [details] [diff] [review]
patch with comments

Looks good.  Thanks for the comments!  Please feel free to land this; I don't know when I'll be able to, but definitely not till Monday.

Comment 8

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

Checked in to trunk.