357538 - [reflow branch] Combobox margin next to float is wrong (bug 342531)

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Mats Palmgren (inactive)]

[reflow branch] Combobox margin next to float is wrong (bug 342531).

STEPS TO REPRODUCE
1. load "Testcase #2" of bug 342531  (attachment 226953 [details])

Error #1:
The "Combobox block" left position is wrong - it should line up with
"Combobox inline" , "List block" etc.

Error #2:
The "Combobox block" width is wrong - it should shrink-wrap (like "List block")

I have an upcoming patch for bug 314939 that fixes error #2 (and other similar cases), hopefully.
This bug is for error #1, which I don't plan to fix in bug 314939.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 2

[Mats Palmgren (inactive)]

(I found more strange combobox widths so I filed bug 357547 on it. The one
case that the patch in bug 314939 seems to fix was probably just by accident.)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 243143 [details] [diff] [review]
Fix

I would prefer if nsIFrame::eReplaced actually means "frame for a replaced
element". In other words, that we add eReplaced to 
nsComboboxControlFrame::IsFrameOfType() instead.
The name eReplaced suggests (to me) all replaced elements have this bit
and it seems awkward to check both bits everywhere.
There are other places which probably would need fixing as well if we
leave nsComboboxControlFrame without eReplaced:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=REFLOW_20060830_BRANCH&root=/cvsroot&mark=1146#1137
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/style/nsComputedDOMStyle.cpp&rev=REFLOW_20060830_BRANCH&root=/cvsroot&mark=2731,2792#2714
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsGenericHTMLElement.cpp&rev=REFLOW_20060830_BRANCH&root=/cvsroot&mark=1204,1232#1187
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=REFLOW_20060830_BRANCH&root=/cvsroot&mark=1423#1419
etc ...

Comment 5

[Mats Palmgren (inactive)]

Attachment: Alt. fix

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 243476 [details] [diff] [review]
Alt. fix

Oh, I see. When the framestate bit got nixed, all the replaced-contains-block stuff but this got the "replaced" bit too.  So yeah, this is the right fix.  Thanks!

Comment 7

[Mats Palmgren (inactive)]

Checked in on REFLOW_20060830_BRANCH at 2006-10-26 18:00 PDT.

(also added bug 342531 to the layout regression test suite on trunk)

-> FIXED