310226 - "new mail alert" popup isn't height enough (got smaller in latest builds)

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Ger Teunis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050927 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050927 Firefox/1.6a1

In the latest builds (aprox 2 weeks or so) the new mail popup isn't height enough.
It lost space below the text / message icon.

Reproducible: Always

Comment 1

[Ger Teunis]

Attachment: Fully popuped up new mail notify popup

The popup should have more space below the text / message picture.

Comment 2

[Scott MacGregor]

I see this too.

Would be great if you could help me narrow down the regression window.

Comment 3

[Ger Teunis]

(In reply to comment #2)
> Would be great if you could help me narrow down the regression window.

Took me a while, but it is introduced in the 20050917 build (the 20050916 worked
fine here)

Comment 4

[Scott MacGregor]

this regression was caused by Bug 305375

Comment 5

[Scott MacGregor]

Attachment: back out the alert.js changes from 305375

this backs out the alert.js changes made in Bug 305375 so the UI regression on
Windows is fixed.

Comment 6

[Jens Bannmann]

The most likely culprit is my workaround for a sizeToContent() bug that adds a
really ugly 1-pixel white line to the right of the window (which is known, but
the cause of which was never tracked down).

Note that this workaround was not neccessary for making it work on Linux (that
bug seems cross-platform); not starting with a 0 pixel-height window was, though.

For now, please back out the changes. I will do some testing soon, and try to
come up with a more selective workaround that doesn't cause any regressions.

Note to self: fix this for SeaMonkey, too.

Comment 7

[Scott MacGregor]

hey Jens,
do you want me to back out all the changes to alert.js or to try just a subset
of those changes?

Comment 8

[Jens Bannmann]

(In reply to comment #7)
> do you want me to back out all the changes to alert.js or to try just a subset
> of those changes?

Wow, I can choose? :-) Then, please just remove these lines:

-  // work around a bug where sizeToContent() leaves a border outside of the content
-  var contentDim = document.getElementById("alertBox").boxObject;
-  var windowDim = document.documentElement.boxObject;
-  window.resizeTo(contentDim.width, contentDim.height);

I'm quite sure the rest does not affect this problem.

Comment 9

[Scott MacGregor]

Attachment: back out just the part of alert.js that's causing the problem.

there's always a choice :)

Comment 10

[[e-mail]]

(In reply to comment #8)
>-  // work around a bug where sizeToContent() leaves a border outside of the
content
>-  var contentDim = document.getElementById("alertBox").boxObject;
>-  var windowDim = document.documentElement.boxObject;
>-  window.resizeTo(contentDim.width, contentDim.height);
Is the outerWidth/Height not the same as the innerWidth/Height for some reason?
Try changing the last line to window.innerWidth = contentDim.width;
(also the windowDim line isn't necessary as I noted in the xpfe version).

Comment 11

[Scott MacGregor]

(In reply to comment #10)
> (In reply to comment #8)
> Try changing the last line to window.innerWidth = contentDim.width;
> (also the windowDim line isn't necessary as I noted in the xpfe version).

Oddly enough, that fixed the problem with the height not being right, but the
alert dialog is now now wide enough. The text runs right up to the right edge of
the alert popup. (On windows)

Comment 12

[Jens Bannmann]

I think we shouldn't blindly set either width or height of the window to that of
the box. After all, the bug in sizeToContent() produces some kind of rounding
error, so it can only be a matter of 1 pixel (is that correct, Neil?). We must
detect the bug unambigiously, and then resize only by that one pixel.

Comment 13

[Scott MacGregor]

jens can you review the backout so we can get this fixed before it's too late
for 1.8, then you and Neil can discuss alternative ways to solve it after the
fact? thanks. 

Comment 14

[Jens Bannmann]

Comment on attachment 197758 [details] [diff] [review]
back out just the part of alert.js that's causing the problem.

>-  // start with a 1px height, because 0 causes trouble with gtk1/2
>-  window.outerHeight = 1;
>+  window.outerHeight = 0;

Starting the animation with a 1 pixel height is neccessary for Linux, and it
shouldn't affect the sizing problem. Please leave that line out of the patch.
With this change, r=me.

Comment 15

[Scott MacGregor]

Comment on attachment 197758 [details] [diff] [review]
back out just the part of alert.js that's causing the problem.

This backs out part of a change we took for Bug 305375 which regressed the new
mail notification for Thunderbird on Windows.

Comment 16

[Scott MacGregor]

fixed branch and trunk using a pixel height of 1.

Comment 17

[Jens Bannmann]

See also bug 310900.

Comment 18

[Worcester12345]

Could this have caused Bugzilla Bug 311256 ?