362594 - [reflow branch] Sonera.fi not rendered properly with reflow branch

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Oleg Romashin (:romaxa)]

REFLOW_20061031_BRANCH

open www.sonera.fi

Top part of web page not rendered properly.

See screen shot of trunk and reflow branch build in attachments.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: More simple testcase

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Simple testcase with small width values

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: reflow log from reflow branch

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Reflow log from trunk

Comment 5

[Boris Zbarsky [:bzbarsky]]

Right.   So the table is overconstrained, it tries to compute min widths for all the cells and then rebalance the table.  The MEW that comes back on trunk seems to be effectively wrong; the MEW on the reflow branch is correct, so the cell is allowed to shrink to 225px (on that first testcase), at which point the images don't fit...

I thought we had something interesting that happened with MEW for nowrap cells... did that go away on branch?

Comment 6

[Bernd]

nowrap overwrites fixed width in quirks mode (it hurts every time I see this)
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLTableCellElement.cpp#374

this is coupled with 

http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableCellFrame.cpp#222

and this got removed at the reflow branch 
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&file=nsTableCellFrame.cpp&rev2=3.383.4.1&rev1=3.383

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible patch

Or should this live in the layout strategy impls?

Comment 8

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

Comment on attachment 247346 [details] [diff] [review]
Possible patch

The main problem with this is that it breaks the invariant that min width <= pref width.  Also, the outer comment seems to be wrong.

Comment 9

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

Although, actually, this should live in the layout strategy, given that we probably don't want this to affect 'width: min-intrinsic' -- that would lead to a contradiction.

Comment 10

[Boris Zbarsky [:bzbarsky]]

OK.  Do we want to do this for fixed layout too?  I would assume "no".

And yeah, the comment is bogus on trunk....

Comment 11

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

(In reply to comment #10)
> OK.  Do we want to do this for fixed layout too?  I would assume "no".

I would also assume "no", given that we probably don't need quirks there given how rarely it's used.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Do it in the strategy

Comment 13

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

Comment on attachment 247360 [details] [diff] [review]
Do it in the strategy

r=dbaron if you fix the comment to mention the "w > minCoord" condition.  (Maybe replace "as the" with "to increase the"?)

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: With better comment

Comment 15

[Boris Zbarsky [:bzbarsky]]

Added reftest for this.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 17

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

Did you mean to mark fixed?

Comment 18

[Boris Zbarsky [:bzbarsky]]

Yes.

Comment 19

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

(In reply to comment #15)
> Added reftest for this.

In the -4 tests, did you mean the difference between border="1" and border="4"?

Also, it might have been good to call them 1-quirks and 1-standards instead of 1 and 3, etc.

Comment 20

[Boris Zbarsky [:bzbarsky]]

> In the -4 tests, did you mean the difference between border="1" and border="4"?

No.  Fixed.

> Also, it might have been good to call them 1-quirks and 1-standards

Ah, good call.  Also fixed.