295282 - About window positioned wrongly

Status: VERIFIED FIXED

(Firefox :: Shell Integration, defect)

Description

[Jamie Diamond]

Trunk builds for the forseable past have displayed the about box further down on
the screen than most Apple and thrid party mac apps. While it looks like you are
aiming for the box to appear dead center, the top should be positioned about a
fourth or so from the top. Could someone please check Apple HIG to see if it's
mandated in there?

Produced on 10.4.1 using:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050523
Firefox/1.0+

Comment 1

[Josh Aas]

Attachment: fix v1.0

This patch works, but I wish I knew of a better way to accomplish this. It is
difficult to figure out the X coord that is the screen width divided by two
then minus half the window's width. I had to hard-code half the window width. I
wanted to do one of 3 other things, but all have problems...

1) Try to do a moveTo(win.screenX, screen.availHeight / 5) after the open()
call. This doesn't work because apparently we can't move the window after
specifying centerscreen, which did the work of figuring out the X coord for us.


2) Do what I did in fix v1.0, but figure out the width/2 dynamically before the
open() call. I don't know how to do that.

3) Get rid of the centerscreen call, and just move the window where it needs to
be by getting its width after the open() call. I don't know how to get its
width, and even if I did, I don't like the idea of moving it since that might
cause the window to flash in the wrong position on open.

Comment 2

[Asaf Romano (gone)]

Comment on attachment 190385 [details] [diff] [review]
fix v1.0

minusing per discussion on irc.

Comment 3

[Josh Aas]

Note for anyone wanting to help: The vertical position of normal about windows
is 1/5 of the way down the screen's available space, within a pixel. That is
what I do in my patch.

Comment 4

[Josh Aas]

Comment on attachment 190385 [details] [diff] [review]
fix v1.0

Potentially reversing the position I had when I spoke with Mano on IRC.
mconnor: ideas? If not, then let's go with this patch.

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 190385 [details] [diff] [review]
fix v1.0

This is possibly imprecise, but its close enough that its not worth agonizing
over.

Comment 6

[Josh Aas]

Attachment: fix v2.0

Better fix using event handler and an anonymous function. We couldn't get the
outerWidth of the window because the content handn't loaded and thus we
couldn't even do sizeToContent() manually. So now, on load we call
sizeToContent and then do our moveTo. This way we don't have to hard-code the
window's width.

Comment 7

[Josh Aas]

Comment on attachment 191270 [details] [diff] [review]
fix v2.0

Canceling review, I accidentally left out something I removed earlier for
testing (resizable=no)

Comment 8

[Josh Aas]

Attachment: fix v2.1

Comment 9

[Josh Aas]

Mano, referring to our IRC conversation: there doesn't appear to be any reason
to use setTimeout() since using that we still need to call sizeToContent(). I
tried. Lets go with 2.1.

Comment 10

[[e-mail]]

Comment on attachment 191273 [details] [diff] [review]
fix v2.1

Why not add the code to aboutDialog.js instead?

Comment 11

[Benjamin Smedberg]

Comment on attachment 191273 [details] [diff] [review]
fix v2.1

What Neil said...

Comment 12

[Josh Aas]

Attachment: fix v3.0

Comment 13

[Asaf Romano (gone)]

Comment on attachment 191387 [details] [diff] [review]
fix v3.0

>Index: aboutDialog.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/aboutDialog.js,v
>retrieving revision 1.6
>diff -u -r1.6 aboutDialog.js
>--- aboutDialog.js	14 Jun 2005 10:21:20 -0000	1.6
>+++ aboutDialog.js	2 Aug 2005 18:44:59 -0000
>@@ -48,6 +48,10 @@
>   button.addEventListener("command", switchPage, false);
> 
>   document.documentElement.getButton("accept").focus();
>+  
>+  // it may not be sized at this point, and we need its width to calculate its position
>+  window.sizeToContent();
>+  window.moveTo((screen.availWidth / 2) - (window.outerWidth / 2), screen.availHeight / 5);
> }
> 
> function uninit(aEvent)

should be under an #ifdef XP_MACOSX. also, remove whitespcae in the line above
the comment.

Comment 14

[Josh Aas]

Attachment: fix v3.1

arg... hopefully this works

Comment 15

[Asaf Romano (gone)]

Comment on attachment 191398 [details] [diff] [review]
fix v3.1

r=mano

Comment 16

[Mike Connor [:mconnor]]

Comment on attachment 191398 [details] [diff] [review]
fix v3.1

ok, there should be a space after chrome, just a nit, but if you're changing
the line, please fix that. ;)

Comment 17

[Josh Aas]

landed fix v3.1