428423 - [FIX]Layout regression with "Change your language" on paypal.com

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: screenshot: broken layout (20080410 nightly)

Recently, we started placing the "Change your language" label on http://www.paypal.com too far to the right, so that it gets overlapped by its combobox.

See screenshots.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot: correct layout (20080308 nightly)

Comment 2

[Daniel Holbert [:dholbert]]

Regressed between 2008-03-14 and 2008-03-15 nightlies.

Bonsai link (for layout subdirectory): http://tinyurl.com/5nodvw

Comment 3

[Daniel Holbert [:dholbert]]

Bonsai link for whole tree: http://tinyurl.com/68oxz8

Comment 4

[Daniel Holbert [:dholbert]]

The testcase I'm about to post shows that this is a change in how we handle fieldset & legend.   --> Regression from bug 404123.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

testcase 1 expected behavior:
  - "left" should be on left edge of blue box
  - "right" should be on right edge of blue box

testcase 1 bad behavior:
  - both "left" and "right" are at right edge of blue box.

FF2 and Trunk nightlies up to 20080314 show expected behavior.
Trunk nightlies after 20080315 show bad behavior.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: reftest: testcase

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reftest: reference

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: This should fix it

We used to give the legend its preferred width no matter what.  Now we want to give it whatever width will "fit", but we were using the block auto-width algorithm when it had display:block, so it ended up too wide.  Using "-moz-fit-content" should give it a narrower width if that's all it needs, but allow it to expand out to the available width as needed.

Comment 9

[Boris Zbarsky [:bzbarsky]]

And Daniel, thanks a ton for the testcases!  They made fixing this literally a 2-minute thing.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: reftest (as patch)

Glad the testcases helped. :)  Here's the reftest as a patch.

(/me tries bz's fix out, brb)

Comment 11

[Daniel Holbert [:dholbert]]

Comment on attachment 315176 [details] [diff] [review]
This should fix it

Description in comment 8 makes sense to me.

Tested patch on my machine, and it makes the reftest pass and also fixes paypal.com

r=dholbert

Comment 12

[Daniel Holbert [:dholbert]]

IMHO, we should definitely get this bugfix into RC1 -- users testing out the release candidate will want their paypal to look right.

Comment 13

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

Comment on attachment 315176 [details] [diff] [review]
This should fix it

I suppose this is OK, but I might prefer doing display:inline!important instead.  Given that we're always constructing the same formatting object for it and using it in the same way, it seems like we ought to make that formatting object always have the same display type.  That said, maybe this is lower risk.  (You've checked that it doesn't cause any change to the display:inline legends, right?)

Comment 14

[Boris Zbarsky [:bzbarsky]]

I did check the display:inline case, and we have some reftests for it already.  It shrinkwraps as things stand, so the behavior doesn't change.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 315176 [details] [diff] [review]
This should fix it

This should be a pretty safe fix to make sure legends don't expand too much when styled as blocks.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 315176 [details] [diff] [review]
This should fix it

a=beltzner, please land with the reftest

Comment 17

[Boris Zbarsky [:bzbarsky]]

Checked in, with tests.