Bug 4379 – negative margins allowed where they should not

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antti Koivisto]

When making window narrow, the navigation bar on top of moneycentral.msn.com
flows out of the view from left. The behavior is different from Gecko/IE where
it stays fully in the view even when window is narrow. The problem is that in
webcore a table with margin-left:auto is allowed to have negative calculated
margin. 

Acutally, it seems that the following rule (css2.1, 10.3.3) is not respected,
even with normal block boxes:

If 'width' is not 'auto' and 'border-left-width' + 'padding-left' + 'width' +
'padding-right' + 'border-right-width' (plus any of 'margin-left' or
'margin-right' that are not 'auto') is larger than the width of the containing
block, then any 'auto' values for 'margin-left' or 'margin-right' are, for the
following rules, treated as zero.

[Antti Koivisto]

Created attachment 3330 [details]
test case for tables

make page narrow to see the problem

[Antti Koivisto]

Created attachment 3331 [details]
test case for blocks

[David Carson]

Created attachment 3347 [details]
margin calculation patch

for boxes in normal flow with non-auto width, treat auto margins as 0 if
width>containing width

[Dave Hyatt]

Yup, this patch is a flawless fix.  Good job.  r=me

[Dave Hyatt]

Comment on attachment 3347 [details]
margin calculation patch

r=me.  Might be worth adding a comment that when you fall into the final else
case, auto margins will end up being 0 when you call minWidth and assign into
m_marginLeft and m_marginRight.

[Antti Koivisto]

Created attachment 3356 [details]
margin calculation patch with comment

(i was apparently logged in as David when submitting the patch)

Added comment

[Darin Adler]

I'm ready to land this, but I'll need to change the test cases into suitable files for the layout-tests directory 
(explaining what they are testing and what good vs. bad results look like).

[Darin Adler]

I tried, but I can't figure out the desired results from the test cases. I'm going to leave this patch until 
someone who does understand them (Antti or Dave perhaps?) turns them into suitable layout tests by 
adding some explanation, or puts notes here enough to let me do that.

[Darin Adler]

I tried, but I can't figure out the desired results from the test cases. I'm going to leave this patch until 
someone who does understand them (Antti or Dave perhaps?) turns them into suitable layout tests by 
adding some explanation, or puts notes here enough to let me do that.

[Antti Koivisto]

Created attachment 3417 [details]
cleaned up test case with an explanation