259389 - cropped account manager UI on Mac OS X

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[(not reading, please use [e-mail] instead)]

cropped account manager UI on Mac OS X

I've got three screen shots.  this is probably a dup.

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: screen shot #1

Comment 2

[(not reading, please use [e-mail] instead)]

Attachment: screen shot #2

Comment 3

[(not reading, please use [e-mail] instead)]

Attachment: screen shot #3

Comment 4

[Asaf Romano (gone)]

We should change the window size on OS X.

Comment 5

[Matthew (lilmatt) Willis]

(In reply to comment #4)
> We should change the window size on OS X.

I'm willing to take this, but I would need to know what to do about the note in
mozilla/mail/components/prefwindow/locale/AccountManager.dtd

<!-- AccountManager.xul -->
XUL/FE DEVELOPERS: DO NOT MODIFY THIS VALUE. It represents the correct size of
this window for en-US.-->
<!ENTITY accountManager.size "width: 55em; height: 44em;">

Should we add an entity for macAccountManager.size and ifdef it?

Comment 6

[Asaf Romano (gone)]

(In reply to comment #5)

> Should we add an entity for macAccountManager.size and ifdef it?

yep

Comment 7

[Matthew (lilmatt) Willis]

Attachment: Screenshot showing problem also on OSX Seamonkey 1.7.3/20040910

This is also a problem on Seamonkey. The screenshot is from OSX Seamonkey
1.7.3/20040910.

There's a couple ways to address this:
1. I can either move AccountManager.xul into Thunderbird from Seamonkey and
then patch
2. I can patch Seamonkey, letting that trickle down to Thunderbird (which will
also require a patch)
3. We can see if Seamonkey or Thunderbird wants to simply change the default AM
width for all platforms from 55em to 60em.

Although the "simplest", I don't think option 3 is the way to go as it may
reintroduce this bug later on when someone fills up AM with widgets on
Linux/Windows again, and we find they don't fit on OSX.

I like option 2 the best. 

I have it fixed in Seamonkey and in Thunderbird, trunk and Aviary. Let me know
what you'd like.

Comment 8

[Matthew (lilmatt) Willis]

Attachment: Screenshot of Thunderbird with patch -- widened window from 55em to 60em

Here is what Thunderbird's AM looks like with the patch

Comment 9

[Asaf Romano (gone)]

Please check all am's panes, "Server Settings" is not the only broken pane.

Comment 10

[Asaf Romano (gone)]

(In reply to comment #9)
> Please check all am's panes, "Server Settings" is not the only broken pane.

(In order to verify that 60em is enough)

Comment 11

[Matthew (lilmatt) Willis]

(In reply to comment #10)
> > Please check all am's panes, "Server Settings" is not the only broken pane. 
> (In order to verify that 60em is enough)

Yes. I checked all of AM's panes in Thunderbird. IMAP, POP, RSS, Local, NNTP --
all good.

Comment 12

[Matthew (lilmatt) Willis]

Attachment: patch widens Account Manager window from 55em to 60em on Mac OS X

This patch widens the Account Manager window on Mac OS X from 55em to 60em. 
This is also a Seamonkey bug.

Comment 13

[Matthew (lilmatt) Willis]

Attachment: AVIARY: patch widens Account Manager window from 55em to 60em on Mac OS X

Same patch, this time against Avairy branch.

Comment 14

[Asaf Romano (gone)]

Comment on attachment 159734 [details] [diff] [review]
AVIARY: patch widens Account Manager window from 55em to 60em on Mac OS X

You don't need to get review two times :-)

Comment 15

[Asaf Romano (gone)]

Comment on attachment 159731 [details] [diff] [review]
patch widens Account Manager window from 55em to 60em on Mac OS X

Thanks for taking this.

Index: mailnews/base/prefs/resources/content/AccountManager.xul
===================================================================
...
>+#ifdef XP_MACOSX
>+<dialog id="accountManager" 
>+        windowtype="mailnews:accountmanager"
>+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>+        title="&accountManagerTitle.label;"
>+        style="&macAccountManager.size;"
>+        persist="screenX screenY"
>+        buttons="accept,cancel,help"
>+        onload="onLoad(event);"
>+        ondialogaccept="return onAccept();"
>+        ondialoghelp="return doHelpButton();">
>+#else
> <dialog id="accountManager" 
>         windowtype="mailnews:accountmanager"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>         title="&accountManagerTitle.label;"
>         style="&accountManager.size;"
>         persist="screenX screenY"
>         buttons="accept,cancel,help"
>         onload="onLoad(event);"
>         ondialogaccept="return onAccept();"
>         ondialoghelp="return doHelpButton();">
>+#endif

As you're changing the style attribute only, you don't need to #ifdef the whole
element. just:

#ifndef XP_MACOSX
style="&accountManager.size;"
#else
style="&macAccountManager.size;"
#endif

>-    content/messenger/AccountManager.xul                                       (base/prefs/resources/content/AccountManager.xul)
>+*   content/messenger/AccountManager.xul       

ha!?


One more note: when you're attaching the new patch, please request the review
from [e-mail]

Comment 16

[Matthew (lilmatt) Willis]

Attachment: TRUNK: revised as per Asaf's comments

Revised as per Asaf's comments.

Comment 17

[Matthew (lilmatt) Willis]

Attachment: AVIARY: revised as per Asaf's comments

Revised as per Asaf's comments.

Comment 18

[Asaf Romano (gone)]

Comment on attachment 159740 [details] [diff] [review]
TRUNK: revised as per Asaf's comments


>-    content/messenger/AccountManager.xul                                       (base/prefs/resources/content/AccountManager.xul)
>+*   content/messenger/AccountManager.xul                                       (base/prefs/resources/content/AccountManager.xul)

Why is it still there?

Comment 19

[Matthew (lilmatt) Willis]

(In reply to comment #18)
> (From update of attachment 159740 [details] [diff] [review])
> 
> >-    content/messenger/AccountManager.xul                                   
   (base/prefs/resources/content/AccountManager.xul)
> >+*   content/messenger/AccountManager.xul                                   
   (base/prefs/resources/content/AccountManager.xul)
> 
> Why is it still there?
> 
Don't we now need to preprocess AccountManager.xul since we've added an ifndef?

Comment 20

[Asaf Romano (gone)]

(In reply to comment #19)

> Don't we now need to preprocess AccountManager.xul since we've added an ifndef?


Oh right, from some reaosn, i tought i had seen it somewhere else.

Comment 21

[Matthew (lilmatt) Willis]

Attachment: Screenshot of Seamonkey with patch - It eliminates cropping

Is this something that should be in 1.8a4 or wait for 1.8a5/1.8b?

Comment 22

[Matthew (lilmatt) Willis]

Chatted with Asaf. 1.8a4 is frozen. Moving to 1.8b.

Comment 23

[[e-mail]]

Great, so nowhere do we have a font metric that works with bold fonts.
This bug is visible on Windows if you set your preferences to bold too.

Comment 24

[Scott MacGregor]

fixed on the aviary 1.0 branch. I need to port it to the trunk. I don't have a
trunk tree handy on this machine at the moment. 

Comment 25

[Asaf Romano (gone)]

Comment on attachment 159740 [details] [diff] [review]
TRUNK: revised as per Asaf's comments

trying to remove extra flag

Comment 26

[Scott MacGregor]

fixed on the trunk now too.