292370 - Gmail - Compose mail layout is a mess

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050429
Firefox/1.0+

repro:
1. Open FF
2. Open Gmail
3. Press Compose mail
it's a mess

regressed today

Comment 1

[Peter van der Woude [:Peter6]]

Attachment: screenshot

Comment 2

[Steve England [:stevee]]

Regression from bug 240276?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Is this pre or post bug 290377? I think that would be a likely candidate.

Comment 4

[Peter van der Woude [:Peter6]]

(In reply to comment #3)
> Is this pre or post bug 290377? I think that would be a likely candidate.

pre, this is with the 20050429 - 0730PDT build

Comment 5

[Peter van der Woude [:Peter6]]

regressed between the 

20050428 1230PDT build
and
20050428 1658PDT build

checkins for the period between the 2 builds
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-28+12%3A00%3A00&maxdate=2005-04-28+16%3A00%3A00&cvsroot=%2Fcvsroot

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Ouch, quite a few layout checkins during that period. CC'ing roc and bz, seeing
as they're responsible for most of them.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

A minimized testcase would be useful here :-)

Comment 8

[Peter van der Woude [:Peter6]]

Attachment: image for testcase

reduced testcase next

Comment 9

[Peter van der Woude [:Peter6]]

Attachment: testcase

in the
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050428
Firefox/1.0+ build the images line up

in the 
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050429
Firefox/1.0+ build they spread out

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Attachment: simpler testcase

The problem appears to be with overflow: auto. In previous builds, it would
hide the extra content and collapse it so that the buttons appear side by side,
but with current builds, it only hides the image contents, but the "hidden"
image still takes up all the space it took when not hidden.

Comment 11

[Erik Fabert]

*** Bug 292465 has been marked as a duplicate of this bug. ***

Comment 12

[Peter van der Woude [:Peter6]]

Attachment: next testcase

(In reply to comment #10)
> The problem appears to be with overflow: auto. In previous builds, it would
> hide the extra content and collapse it so that the buttons appear side by
side,
> but with current builds, it only hides the image contents, but the "hidden"
> image still takes up all the space it took when not hidden.
It actually only takes the remaining space of the image, not the full length of
the strip.

Comment 13

[Peter van der Woude [:Peter6]]

->?1.8b2
->?1.1

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The MEW and max-width need to be adjusted for the element's style.

Comment 15

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

Comment on attachment 182360 [details] [diff] [review]
fix

This should also account for min-width and max-width in
AdjustIntrinsicContentWidthForStyle.  You should probably use the values from
the style system.  With that, r+sr=dbaron.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

I just applied the current patch to my source locally and did a build. It does
NOT fix the problem. Maybe the addition of dbaron's suggestions will?

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Did it fix the testcases here?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix #2

OK, this adds code to handle min/max-width as well. But I had to break
intrinsic-min-width and intrinsic-width into separate functions because now
they have to be handled differently. I'm not sure what to do with
intrinsic-width and percentage widths, so I'm not do anything there (I don't
think any of our code does anything special there). This does fix gmail for me.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(In reply to comment #17)
> Did it fix the testcases here?

If the icons are supposed to be directly next to each other in the test cases,
then no, none of them are fixed. All three still have large gaps between the icons.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There are supposed to be some gaps between the icons. Anyway, fix #2 gives
identical output to FF1.0.1 in these testcases and gmail.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Attachment: What I'm seeing

Just applied patch #2 and did a new build. This is what I'm seeing at GMail.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suspect you're not applying and building properly.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

(In reply to comment #22)
> I suspect you're not applying and building properly.

Patch definitely applied OK (I've opened up the three source files and manually
confirmed that the changes were made).

I just cleared out my build directory and am doing a build everything, so we'll
see how it goes.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Any idea why I'd be getting this error now when I try to compile?
---------------------------
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(101) : warning C4355:
'this' : used in base member initializer list
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(423) : error C2039:
'LeftRight' : is not a member of 'nsMargin'
        ../../dist\include\gfx\nsMargin.h(44) : see declaration of 'nsMargin'
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(426) : error C2039:
'LeftRight' : is not a member of 'nsMargin'
        ../../dist\include\gfx\nsMargin.h(44) : see declaration of 'nsMargin'
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(434) : error C2039:
'LeftRight' : is not a member of 'nsMargin'
        ../../dist\include\gfx\nsMargin.h(44) : see declaration of 'nsMargin'
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(437) : error C2039:
'LeftRight' : is not a member of 'nsMargin'
        ../../dist\include\gfx\nsMargin.h(44) : see declaration of 'nsMargin'
m:/mozilla\mozilla\layout\generic\nsGfxScrollFrame.cpp(871) : warning C4355:
'this' : used in base member initializer list
make[4]: *** [nsGfxScrollFrame.obj] Error 2
make[4]: Leaving directory `/cygdrive/m/mozilla/mozilla/foxbuild/layout/generic'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/m/mozilla/mozilla/foxbuild/layout'
make[2]: *** [tier_9] Error 2
make[2]: Leaving directory `/cygdrive/m/mozilla/mozilla/foxbuild'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/m/mozilla/mozilla/foxbuild'
make: *** [alldep] Error 2

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: extra chunk of fix #2

Sorry, I forgot to attach this other diff (very simple helper functions in
nsMargin).

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Confirming that fix #2 + the extra chunk for fix #2 builds OK and fixes the
GMail formatting.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 182368 [details] [diff] [review]
fix #2

fix this serious layout regression...

Comment 28

[Mike Kaply [:mkaply]]

Comment on attachment 182368 [details] [diff] [review]
fix #2

a=mkaply

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 30

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

FWIW, you may also be interested in seeing how I did this on the reflow branch:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsLayoutUtils.cpp&rev=3.34.4.1&mark=467-522#474

Comment 31

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using build 2005-05-08-05 on Windows XP Seamonkey trunk.