Dialogue boxes too large for 1024x768 resolution

Reporter, what build are you using? There was a regression, recently fixed, in
which the dialog in question was sized too large.

I am using the most recent build.

WORKSFORME
Platform: PC
OS: Linux 2.2.16
Mozilla Build: 2001010221

Reporter try with a new profile my bet is that will fix it.

I have a lot invested in the profile I've got. Besides, why should that fix it?
Aren't window designs, sizes and positioning specified in XUL files and C++
code? What does this have to do with a new profile?

I downloaded Mozilla onto another computer, created a new profile, opened up
Address Book and clicked New Card. Once again, the window sizing and positioning
were terrible for 800*600 and 640*480 resolutions. So creating a new profile
does not help, unfortunately.
Please take a look at the screenshot page I prepared in order to demonstrate
what I am talking about, and try it also on your own computer.
<a href="www.caseyperkins.com/newcard.html">www.caseyperkins.com/newcard.html
</a>
Thanks.

Marking NEW as per reporters comments.

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

os/platform -> all, since the duplicate is reported on a Mac running System 9.

reassigning to chuang.

QA-assign-to fenella.

It seems like fixing would be a simple matter of x and y positioning and sizing.
I do some GUI programming (in Java) and that would be how I would do it there.
Can it be that different in C++/XUL?

As an example of centering the box in reference to the Mozilla window:
int height=450;
int width=300;
newCardBox.setSize(width,height);
int setLeft=(((mozWindow.getWidth()-width)/2)+mozWindow.getX());
int setRight=(((mozWindow.getHeight()-height)/2)+mozWindow.getY());
newCardBox.setLocation(setLeft,setRight);
newCardBox.show();

Yes, its Java pseudo-code, not C++, but the logic is good. Can't something
similar to this be implemented for the New Card box?

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

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

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

Marking nsbeta1 since running at a resolution of 640x480 makes the new card
dialog unusable. It's so large that the bottom half of the dialog cannot be
seen, there is no way to traverse to the bottom to select the OK/Cancel buttons.
Resolution 600x800 is also too large but I can atleast move the dialog to access
the OK/Cancel buttons.

reassigning to racham.

Is this something hard to fix? I would think not. If not, please move this from
future to something sooner. There is simply no good reason for this to be so tall.

At 800 X 600 in Linux Mandrake KDE and in OS/2, the new card window is taller
than the space between the edge of the screen and the toolbar. In W2K at 800 X
600, it is only slightly less tall than that space, and opens way below 0,0,
leaving the OK & cancel buttons offscreen by default. In OS/2, it opens each
time with the titlebar way above the top of screen. See screenshot
http://bugzilla.mozilla.org/attachment.cgi?id=91249&action=view and bug 67913.

Simply right adjusting the currently left adjusted 'add to . . .' or the
'name,address,other' tabs would allow substantial vertical compaction without
losing [the excessive] whitespace between any entry fields.

The currently excessive whitespace means this could easily be made much shorter
than it currently is and accomodate minimum screen resolutions.

Created attachment 112016
screenshot of new card in KDE, with KDE toolbar, at 800x600

Attaching screenshot of new card in KDE, with KDE toolbar, at 800x600.

mass re-assign.

As of "Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616"
I still see this bug (no ok and no cancel) on Win2k with a 3 window high
taskbar. This needs to be fixed and should be simple XUL I think.

As of "Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616"
I still see this bug (no ok and no cancel) on Win2k 800x600 with a 3 window
high taskbar. This needs to be fixed and should be simple XUL I think.

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

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

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

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

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

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

This bug should be assigned a priority -- it affects the visually handicapped like me, and is a really ugly propblem on the "new card" pagelength problem reported as a "duplicate" to this report.

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

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

Created attachment 234396
633px tall new card screenshot 1024x768x96DPI (KDE)

~633px is the minimum required height @ 96 DPI

Created attachment 234397
KDE fullscreen screenshot 800x600x96DPI

The problem is actually worse now than when attachment 112016 was affixed. DPI is now max(96dpi,system setting), which means unless a user has taken the trouble via user.js or about:config to force Gecko to use ~84 DPI or less, the card can't fit in the available vertical space, even with the taskbar hidden.

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

 Ok, but is there a solution to this problem? Reassigning it to this bug number just moves it around.

There are a couple of solutions discussed in the Mozillazine forums:
http://forums.mozillazine.org/viewtopic.php?t=390988
http://forums.mozillazine.org/viewtopic.php?t=455604

These have to be manually added to a UserChrome.css file in each user profile though -- doesn't address the underlying problem.

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

Comment on attachment 344934
New Card Layout - Patch now with seamonkey and thunderbird plus themes

>diff -r 785c708554f7 mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd
>--- a/mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd Mon Oct 27 14:33:43 2008 +0100
>+++ b/mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd Mon Oct 27 17:33:41 2008 +0100
>@@ -17,18 +17,18 @@
> The Initial Developer of the Original Code is
> Netscape Communications Corporation.
> Portions created by the Initial Developer are Copyright (C) 1998-1999
> the Initial Developer. All Rights Reserved.
>
> Contributor(s):
>
> Alternatively, the contents of this file may be used under the terms of
>- either the GNU General Public License Version 2 or later (the "GPL"), or
>- the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+ either of the GNU General Public License Version 2 or later (the "GPL"),
>+ or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),

Please don't change this. The license has to be a set format and wording as specified here: http://www.mozilla.org/MPL/boilerplate-1.1/

>@@ -75,49 +75,50 @@
> <!ENTITY PreferMailFormat.label "Prefers to receive messages formatted as:">
> <!ENTITY PreferMailFormat.accesskey "r">
> <!ENTITY PlainText.label "Plain Text">
> <!ENTITY HTML.label "HTML">
> <!ENTITY Unknown.label "Unknown">
> <!ENTITY ScreenName.label "Screen Name:">
> <!ENTITY ScreenName.accesskey "S">
>
>-<!ENTITY allowRemoteContent.label "Allow remote images in HTML mail.">
>+<!ENTITY allowRemoteContent.label "Allow remote images.">
> <!ENTITY allowRemoteContent.accesskey "w">
>+<!ENTITY allowRemoteContent.tooltip "In HTML messages it is possible to embed images from remote sources. Opening such a message will open a connection to this external source. This allows to track the reading of the message. This check box will allow such external embedded images in HTML messages.">

Slightly better wording:

"In HTML messages it is possible to embed images from remote sources. Opening such a message will open a connection to this external source. This may allow tracking of the message being read. Checking this box will allow such external embedded images in HTML messages from this contact."

> <!ENTITY Custom4.label "Custom 4:">
> <!ENTITY Custom4.accesskey "4">
>-<!ENTITY Notes.box "Notes">
>-<!ENTITY Notes.accesskey "N">
>+

No need for a blank line here, as long as there's just one new line after the last > we're fine.

> <tabpanels id="abTabPanels" flex="1">
>
> <!-- ** Name Tab ** -->
>- <vbox index="name" flex="1">
>- <groupbox flex="1">
>- <caption label="&Name.box;"/>
>+ <tabpanel>
>+ <vbox>
>+ <hbox index="name" flex="1">
>+ <vbox>
> <vbox>

If you look at the existing code, <tabpanel> isn't necessary.

You also have an extra <vbox> in here (after the <hbox>) that you don't need.

Unfortunately, that means you still have an extra box elem...

Created attachment 345282
New Card layout - Updated patch including the comments from Standard8

This patch includes the comments from standard8. I changed the indention and removed useless tabs and boxes. There was no difference in the patch size between -U8 and -U8 -w, so I only included the -U8 diff. The files

mail/locales/en-US/chrome/messenger/addressbook/abCardOverlay.dtd
suite/locales/en-US/chrome/mailnews/addressbook/abCardOverlay.dtd

are identical. However there is a (small) difference in the license text in the original code. So I had to choose one to make the files really identical. I took the one from the template that standard8 mentionend. Thanks for the review!

(In reply to comment #79)
> >- either the GNU General Public License Version 2 or later (the "GPL"), or
> >- the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
> >+ either of the GNU General Public License Version 2 or later (the "GPL"),
> >+ or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>
> Please don't change this. The license has to be a set format and wording as
> specified here: http://www.mozilla.org/MPL/boilerplate-1.1/
I reversed this. Now it complies to the template.
>
> Slightly better wording:
>
> "In HTML messages it is possible to embed images from remote sources. Opening
> such a message will open a connection to this external source. This may allow
> tracking of the message being read. Checking this box will allow such external
> embedded images in HTML messages from this contact."
I took your wording.
> >-<!ENTITY Notes.accesskey "N">
> >+
>
> No need for a blank line here, as long as there's just one new line after the
> last > we're fine.
Done.
> If you look at the existing code, <tabpanel> isn't necessary.
>
> You also have an extra <vbox> in here (after the <hbox>) that you don't need.
>
> Unfortunately, that means you still have an extra box element, so you need to
> re-indent the lines it encloses by two spaces.
>
> Once you have done this, please attach the full patch and a diff -w patch.
I removed the <tabpanel> and the useless <vboxes> and <hboxes>. The diff -U8 -w showed no difference to the diff -U8, so I only added the full patch.
>
> As you're now touching these lines, please can you wrap accesskey onto the next
> line and align it with the start of control. You may need to wrap the class
> attribute.
I did the wraps to have less line length overall.
>
> Mostly this is looking fine, I've verified it all seems to work on SM (both
> themes) & TB on Mac, I'll check it on Linux once you update the patch.
Thanks for review!

(In reply to comment #81)
> > Once you have done this, please attach the full patch and a diff -w patch.
> I removed the <tabpanel> and the useless <vboxes> and <hboxes>. The diff -U8 -w
> showed no difference to the diff -U8, so I only added the full patch.

Unfortunately diff -w doesn't always work in Mercurial, see https://developer.mozilla.org/en/Mercurial_FAQ#How_do_I.c2.a0get_a_diff_-w.3f for more info.

I'm just reviewing the patch though, so don't worry about adding a diff -w straight away.

Comment on attachment 345282
New Card layout - Updated patch including the comments from Standard8

> <tabpanels id="abTabPanels" flex="1">
>+ <!-- ** Name Tab ** -->
>+ <!-- The following vbox contains two hboxes top: Name/Email/Phonenumber bottom: Email prefs. -->
>+ <vbox>
>+ <!-- This hbox contains two vboxes left: Name/Email, right: Phonenumbers -->
>+ <hbox index="name" flex="1">

I think the index/flex attributes should be on the <vbox> element above.

I think the business vbox also needs an index.

Otherwise r=me with those fixed. You'll need an sr for this patch as well.

Created attachment 346231
Updated patch - removed the index attributes in abCardOverlay.xul

I removed the index attributes as they are useless. I added ids to the for tabs. They may be useful for future overlays.

Comment on attachment 346231
Updated patch - removed the index attributes in abCardOverlay.xul

git-apply complained of 32 whitespace errors, of which at least 25 are new.
If you can't find them try /msg firebot review 346231 (note: it's designed to review C++ so some of its other quibbles aren't always appropriate.)

I thought the display looked a little crowded; the current version at least has those groupboxes that break up the dialog into manageable pieces.

>+<!ENTITY allowRemoteContent.tooltip "In HTML messages it is possible to embed images from remote sources. Opening
>+such a message will open a connection to this external source. This may allow
>+tracking of the message being read. Checking this box will allow such external
>+embedded images in HTML messages from this contact." >
I don't know how common wrapping long entities is, but you've wrapped the text in the entity rather than the text in the file, which looks odd. Also speaking of whitespace errors you can lose the space before the >

> .CardEditWidth {
>- width: 25em;
>+ width: 21em;
Is it possible to change these widths to the equivalent in ch?
(This affects themes/platforms that use a bold font)

> .alignBoxWithFieldset {
> margin-left: 6px;
> margin-right: 5px;
> }
You don't use this any more. (Both comments apply for all themes of course.)

>+ <hbox id="nickNameContainer">
>+ <spacer flex="1"/>
>+ <label control="NickName" value="&NickName.label;"
>+ accesskey="&NickName.accesskey;" class="CardEditLabel"/>
>+ <hbox class="CardEditWidth">
>+ <textbox id="NickName" flex="1"/>
> </hbox>
> </hbox>
Just picking on this one because it was easy to extract on the diff, but this applies to most of the fields in this window.
Rather than setting the top margin in the CardEditLabel class, I think the outer hbox needs an align="center" so the labels line up properly with the textboxes.
If you wanted to, you could get rid of the <spacer> by using <label flex="1"> and adding text-align: right; to the CardEditLabel class.

>- <caption>
>- <label control="Notes" value="&Notes.box;" accesskey="&Notes.accesskey;"/>
>- </caption>
This caption was important as it labelled the Notes control...

Created attachment 346254
Updated patch - included comments from neil

This patch includes changes based on the comments from Neil.
* I removed the cardEditLabel class altogether. The alignment of the labels is done via the align="center" attributes.
* Changed the css unit from em to ch
* Added the notes label
Thanks for the review!

Created attachment 346259
Updated patch - included comments from neil and whitespace

This patch includes changes based on the comments from Neil.
* I removed the cardEditLabel class altogether. The alignment of the labels is
done via the align="center" attributes.
* Changed the css unit from em to ch
* Added the notes label
* Reduced the whitespace and longline style bugs
Thanks for the review!

(In reply to comment #85)
> (From update of attachment 346231 [details])
> git-apply complained of 32 whitespace errors, of which at least 25 are new.
> If you can't find them try /msg firebot review 346231 (note: it's designed to
> review C++ so some of its other quibbles aren't always appropriate.)
Thanks for the hint. I removed the whitespace and longline errors but there are 4 left in the current patch. I have changed it locally here for the moment.
>
> I thought the display looked a little crowded; the current version at least has
> those groupboxes that break up the dialog into manageable pieces.
I removed the groupboxes on purpose because they consume quite some space. Because I address is now split in home and business, that one is obsolete. For the phone numbers and Emails on the first tab, I compared it to Outlook where they also have it like this. So in the end I removed them only because of the space.
> >+<!ENTITY allowRemoteContent.tooltip "In HTML messages it is possible to embed images from remote sources. Opening
> >+such a message will open a connection to this external source. This may allow
> >+tracking of the message being read. Checking this box will allow such external
> >+embedded images in HTML messages from this contact." >
> I don't know how common wrapping long entities is, but you've wrapped the text
> in the entity rather than the text in the file, which looks odd. Also speaking
> of whitespace errors you can lose the space before the >
I split it a little differently to have no line longer than 80 but I left it essentially.
>
> > .CardEditWidth {
> >- width: 25em;
> >+ width: 21em;
> Is it possible to change these widths to the equivalent in ch?
> (This affects themes/platforms that use a bold font)
Done.
>
> > .alignBoxWithFieldset {
> > margin-left: 6px;
> > margin-right: 5px;
> > }
> You don't use this any more. (Both comments apply for all themes of course.)
Removed it.
>
> >+ <hbox id="nickNameContainer">
> >+ <spacer flex="1"/>
> >+ <label control="NickName" value="&NickName.label;"
> >+ accesskey="&NickName.accesskey;" class="CardEditLabel"/>
> >+ <hbox class="CardEditWidth">
> >+ <textbox id="NickName" flex="1"/>
> > </hbox>
> > </hbox>
> Just picking on this one because it was easy to extract on the diff, but this
> applies to most of the fields in this window.
> Rather than setting the top margin in the CardEditLabel class, I think the
> outer hbox needs an align="center" so the labels line up properly with the
> textboxes.
> If you wanted to, you could get rid of the <spacer> by using <label flex="1">
> and adding text-align: right; to the CardEditLabel class.
I removed the CardEditLabel class and did the alignment with the attribute.
>
> >- <caption>
> >- <label control="Notes" value="&Notes.box;" accesskey="&Notes.accesskey;"/>
> >- </caption>
> This caption was important as it labelled the Notes control...
I added the label again.

Thanks for the review!

Created attachment 346269
Updated patch - modified the accesskeys

Modified the access keys and removed the remaining 4 whitespace errors.
Should be possible to navigate throught the dialog with keys.

Comment on attachment 346269
Updated patch - modified the accesskeys

> <!ENTITY WorkZipCode.label "ZIP/Postal Code:">
>-<!ENTITY WorkZipCode.accesskey "P">
>+<!ENTITY WorkZipCode.accesskey "z">
Nit: Prefer to match the case of the accesskey to the label's character.

Created attachment 346430
Updated patch - now with capital access keys

Just changed some access keys to capital letters.

Checked in: http://hg.mozilla.org/comm-central/rev/4ae62e9a43c3

Thanks for all your work on this Fritz.

(In reply to comment #92)
> Checked in: http://hg.mozilla.org/comm-central/rev/4ae62e9a43c3
>
> Thanks for all your work on this Fritz.
Thank you, Clark, Neil and the others for your reviews and help!
Nice environment here.

<!ENTITY Birthday.label "Birthday:">
<!ENTITY Birthday.accesskey "B">
<!ENTITY Year.emptytext "Year">
<!ENTITY Or.value "or">
<!ENTITY Age.emptytext "Age">

I suggest to move these to the "Private" section to reflect the situation in UI.

Created attachment 346645
Move birthday section in abCardOverlay.dtd as proposed in Comment #94

This is a small patch to rearrange the entities in the abCardOverlay.dtd file. The birthday section is moved from the "Other Tab" to the "Private Tab" entries to be aligned with the display in the user interface address card dialog. This patch does not change behaviour.

Comment on attachment 346645
Move birthday section in abCardOverlay.dtd as proposed in Comment #94

I'm not sure its absolutely vital that we do this. However, seeing as the patch is here and they are reasonably well organised in the file, we can take it.

(In reply to comment #96)
> (From update of attachment 346645 [details])
> I'm not sure its absolutely vital that we do this. However, seeing as the patch
> is here and they are reasonably well organised in the file, we can take it.
The changes has been checked in: http://hg.mozilla.org/comm-central/rev/eb6e30c22d74
Thanks! Friedrich

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

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

The bug is still in:
thunderbird-2.0.0.19.en-US.win32.installer.exe

I guess the nightly's aren't.

The bugfix is in the 3.x series which isn't compatible with lightning.
Does anyone know when the bugfix will make into a nightly?

BTW, Thanks for fixing the bug : )

(In reply to comment #100)
> The bug is still in:
> thunderbird-2.0.0.19.en-US.win32.installer.exe

Patches like this will never make it into released major versions because the updates are for security and stability updates.

> The bugfix is in the 3.x series which isn't compatible with lightning.
> Does anyone know when the bugfix will make into a nightly?

If you mean a 2.x nightly then the answer is never. If you mean a 3.x nightly then it is already there and there are lightning versions available for the 3.x nightlies.

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

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

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

All the discussions regarding what fields go where - surely making the Edit Card scrollable is the best answer!!

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

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

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

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

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

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