Closed Bug 123370 Opened 23 years ago Closed 21 years ago

Entering a new profile name pushes the buttons out of the dialog box

Categories

(SeaMonkey :: Startup & Profiles, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ktrina, Assigned: ccarlen)

References

Details

(Keywords: fixed1.4.1, polish)

Attachments

(9 files, 7 obsolete files)

21.88 KB, image/png
Details
2.51 KB, patch
timeless
: review+
dmosedale
: superreview-
Details | Diff | Splinter Review
5.88 KB, image/png
Details
5.95 KB, image/png
Details
1.39 KB, patch
Details | Diff | Splinter Review
6.95 KB, image/png
Details
829 bytes, patch
ccarlen
: review+
jag+mozilla
: superreview+
Details | Diff | Splinter Review
27.99 KB, image/png
Details
1.99 KB, patch
ccarlen
: review-
Details | Diff | Splinter Review
Steps to reproduce:
1. Install MacOSX build 2001-02-04-08-trunk
2. Open the profile manager and click Create Profile then Next
3. Enter New Profile Name 

Results:
The Choose Folder, User Default and Region Selection buttons get pushed to the
right of the dialog box 

Expected results:
The location path should wrap
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.1
Platform -> All (reproduced on Windows XP)
OS: MacOS X → All
Hardware: Macintosh → All
on my Mac OS X box, the buttons start pushing off the screen after only the
6th character. It takes 23 only characters to push the buttons entirely off the
screen and make the dialog unusable. In my opinion, the dialog is unusable after
only 14 characters because so much of the button is obscured.
I think the problem is worse than it appears to you:
You have no screenshot attached, but from the comments I think at least in the
beginning the buttons are completely visible on english Windows versions.

That is *not* true with at least german Windows 2000 (and probably more
languages as well): as you can see on the screenshot, the buttons are cut off
already at the very beginning! and no matter what profile you enter, the
buttons are *never* completely visible.
(and I think this does not look very professional to a new mozilla user
creating his first profile...)

The reason is probably the different name of the standard path for user data in
other language versions of Windows. "application data" contains a space and
this is used for a wrap, I suppose. "Anwendungsdaten" does not contain a space
and therefore there is a quite long string not wrapped (and the window is not
resizable, so I never saw the complete labels of the buttons...).

It might be possible that in other languages the buttons are not visible at
all.
So I propose to raise this bug's severity and address the problem really soon.
Attached patch Proposed patch (obsolete) — Splinter Review
Keywords: patch, polish, review, ui
Blocks: 146490
Blocks: patchmaker
Attachment #98320 - Flags: review+
Attachment #98320 - Flags: superreview?(bzbarsky)
uh...

from what I see, bug 174121 might be just a dupe of this one....
*** Bug 174121 has been marked as a duplicate of this bug. ***
Just played around a bit with this patch and it looks very nice - thanks, Neil!
The buttons stay visible all the time.

Since the patch is here for 4 Months now and the first impression of Mozilla
users with german Windows2000/XP is still a box with only partly visible
buttons, could somebody please superreview it? 

Review has been done almost 4 Months ago...

Currently it's on bzbarsky's sr= list, but I know this queue is quite long (I
currently count 17 sr= requests). Maybe someone else might do the superreview?
Thanks very much!
(And sorry for the spam but maybe this patch has simply been forgotten...)
Patch proposed, tested, and reviewed, could this be considered for 1.3 ?
If the path to your profile is long, you can´t use long names, because the
buttons are sliding out of reach.
The bug is trivial, but I wouldn´t see it as polish, because functionality is
sincerely reduced, at least for german users insisting on long profile names.
Flags: blocking1.3?
We're not going to hold the release for this but we'll consider a fully reviewed
patch if it's low-risk. 
Flags: blocking1.3? → blocking1.3-
Attachment #98320 - Flags: superreview?(bzbarsky) → superreview?(dmose)
Comment on attachment 98320 [details] [diff] [review]
Proposed patch

As discussed with Neil in IRC, this needs at least more commentary, possibly a
bit of restructuring as well.
Attachment #98320 - Flags: superreview?(dmose) → superreview-
*** Bug 196825 has been marked as a duplicate of this bug. ***
*** Bug 197028 has been marked as a duplicate of this bug. ***
Attachment #98320 - Attachment is obsolete: true
Attachment #117096 - Flags: superreview?(dmose)
Attachment #117096 - Flags: review?(timeless)
Attachment #117096 - Flags: review?(timeless) → review+
Comment on attachment 117096 [details] [diff] [review]
Updated for review

Sorry for not getting to this sooner.  The comments are helpful; thanks.  It
would be nice to have a little vertical whitespace as well.

>+  // use this funky regexp to split the text up
>+  // split long words in case someone names their profile
>+  // llanfairpwllgwyngyllgogerychwyrndrobwyll-llantysiliogogogoch
>+  // magic word length constant pulled from chatzilla
>+  var words = aValue.match( /(\b\w{1,20}\b)|.\s*/g );

20 seems to be an arbitrary constant.  Rather than looking at the number of
characters in the string, isn't the right thing to do actually figure out what
the width of aValue is rendered with the current font?	I'm thinking about CJKV
fonts in particular where the average character width is likely to be
significantly greater than with latin fonts.  If that's too hard, I can be
convinced that this is a reasonable hack, but better justification than "that's
how chatzilla does it" would be nice.
Attachment #117096 - Flags: superreview?(dmose) → superreview-
I suppose that 1.4 will last for some time, so it would be nice to get this in
at least as a quick&dirty hack, if it doesn´t break more than what is already
broken.
I would rather prefer to get the second patch in and (not) improved later,
than not to get the second patch in and also not improved later.
Later is mozilla 1.5, and I doubt anybody will then do anything in the suite.
re-reading comment #14, the negative superreview from 12 weeks ago:

20 seems to be an arbitrary constant.....
....
If that's too hard, I can be convinced that this is a reasonable hack, ....

I assume this patch will only improve the situation, not worsen it.
Lack of this patch gives a very bad impression to new users using mozilla the
first time, and the first 20 seconds are important to a new relationship ;-)

could this be considered once more, checked in while waitng for the blocker bugs?
Would be nice to have it in 1.4, not 1.4.1 sometime, if there will be one.
Flags: blocking1.4?
Worse than ever!
Today I wanted to create a new profile, and didn´t see buttons.
When I deleted the name 'Default User', to enter a new one, some buttons got
visible, but I couldn´t remember what they should mean.
Here you see, that after deleting 'Default User', buttons get partly visible,
but can´t be read.
The path shown is a standard windows path, with a three-letter-username.
If I would have used a six-letter-name, the buttons wouldn´t have get visible.
OS is Win98SE.

Now imagine, you are a new user, installing mozilla first time, and want to use
a personal name for your profile, or create a second one.
If you don´t see the buttons, all is well, but if you see them, how to get info
what they will do? There is no help for this.

Raising Severity to major because of this.
Attached patch Alternative, simpler approach (obsolete) — Splinter Review
This patch adresses the _button_ problem differently:
It moves them out of the directory-<hbox> down next to the "Click Finish..."
label.

Motivation: this label has a fixed length in english and is very likely to be
similarly short in other languages. If not, it can still use several lines;
there is plenty of room next to the buttons.

Remarks: the "width: 28em" of this label makes the buttons appear at the
correct place: the right. The containing iframe is hardcoded to be 30em wide,
so this fixed 28em are not about to cause problems anyhow.
The <hbox id="dirbox"> seems unneeded now. This id does not show up anywhere
else in lxr, so this hbox could be removed imo. But I'm not completely sure, so
I left it in for others to decide.

Problem: the displayed directory label is still unchanged, this means it might
still be too long to show completely. Solving this problem requires splitting
the string as proposed in the other (refused) patches.
But obviously main developers and reviewers do not recognize how ugly the
problem is with non-english Windows versions and do not care much about the
large number of people using them, therefore this limited approach trying to
satisfy both sides...

Advantages: My solution solves the main problem - the invisible buttons - with
very small changes (which might make it more "reviewable", maybe even
"approvable"...) and does not prevent solving the problem completely (as in the
previous patches). In contrary: it leaves more horizontal space to the
directory label, thus reducing it's number of necessary linebreaks.
Comment on attachment 125913 [details] [diff] [review]
Alternative, simpler approach

Requesting r= from timeless and sr= from dmose (as they had a look at the
previous patches).
This fixes the main problem, maybe even in time for 1.4 (??), but in all cases
still leaves room for an approach as in the patches by Neil.
Please also see my previous comment.
Attachment #125913 - Flags: superreview?(dmose)
Attachment #125913 - Flags: review?(timeless)
Attached image Screenshot of the last patch (obsolete) —
How it looks like with this patch: the buttons are at the bottom of the box,
cannot be pushed anywhere by the directory label.
Comment on attachment 125913 [details] [diff] [review]
Alternative, simpler approach

the original real problem (really long paths) still needs to be addressed (your
picture shows /something/ being cut off). but imo there's nothing wrong with
this change.
Attachment #125913 - Flags: review?(timeless) → review+
timeless: I said that in comment #19.

The problem this patch _can_ cause is that the buttons are pushed _down_ if the
directory name is too long. "Too long" being four full lines, which in my tests
was around 250 characters.
But only one button is slightly covered then while all three are already mostly
invisible when the path is not changed at all, so this is still definitely a
major improvement of the buttons problem.
conrad: could we change the layout of the buttons to horizontal? =)
Assignee: ben → ccarlen
Status: ASSIGNED → NEW
Target Milestone: mozilla1.1alpha → ---
Attached patch Buttons horizontally aligned (obsolete) — Splinter Review
Timeless: you mean something like this?

Buttons horizontally next to each other under the directory label.
Comment on attachment 125946 [details] [diff] [review]
Buttons horizontally aligned

Requesting r= from timeless. Feel free to move sr= request to this patch if you
like this better.

The <vbox id="dirbox"> may still be removed if it serves no purpose. Its id is
never being used in lxr.
Attachment #125946 - Flags: review?(timeless)
Attached image Screenshot from last patch (obsolete) —
Last patch looks like this. It's your choice.
Comment on attachment 125947 [details]
Screenshot from last patch

.
Attachment #125947 - Attachment is patch: false
Attachment #125947 - Attachment mime type: text/plain → image/png
Attachment #125947 - Attachment description: Screenshot from lastt patch → Screenshot from last patch
The horizontal layout is a big improvement over what we have now. I think the
grouping could be better, though. The two buttons on the left are associated,
but have nothing to do with the 3rd. Does "Use Default" mean the
default directory or the default region (to the uninitiated, 1st time user
that's not too bright)? Can the "Region Selection" button be right-aligned, or
separated in some way? 
ccarlen, so more like this?

If the spacer would be flex="1" the right button would be right-aligned which
looks nicer, BUT it also would make it disappear if the directory label is too
long.
I also could fix its position on the very right of the visible area, BUT what
if some translation of its label is a few chars longer?
So I used a spacer with fixed width; translations may shift the buttons to the
right (or left), but there is plenty of room for longer labels now.
Attachment #125913 - Attachment is obsolete: true
Attachment #125914 - Attachment is obsolete: true
Attachment #125946 - Attachment is obsolete: true
Attachment #125947 - Attachment is obsolete: true
Attachment #125913 - Flags: superreview?(dmose)
Attachment #125946 - Flags: review?(timeless)
However, I did not like the button placement of the previous patch: the
directory modification buttons are the most likely ones to be used and I guess
from an usability perspective they should appear on the right (that's where
they "always" are and I felt they had to be when I saw the previous patch).
So this patch puts the "Region Selection" button to the left, but leaves
everything else intact (grouping,...).
How the last patch looks like.

Read comment #30 on why the directory buttons cannot be placed at the very
right.
Also look at the attachments in bug 146490 for the larger buttons on MacOS;
these alone already would push the right button out of the visible area if it
was positioned to be at the very right border.
Let "dirbox" stay a <hbox>, no need to change it to <vbox> anymore.
Sorry for the spam!
Attachment #126006 - Attachment is obsolete: true
Comment on attachment 126008 [details] [diff] [review]
Simplified patch (does the same as attachment 126006 [details] [diff] [review] above)

Requesting r= from ccarlen and sr= from dmose.
Please feel free to change requests to the patch without reversal or other
reviewers if appropriate.
I'll be away for a few days soon, so please also request approval1.4 if timing
still allows a chance to get it in 1.4.
Thanks!

BTW: I have no checkin permission, so...
Attachment #126008 - Flags: superreview?(dmose)
Attachment #126008 - Flags: review?(ccarlen)
Comment on attachment 126008 [details] [diff] [review]
Simplified patch (does the same as attachment 126006 [details] [diff] [review] above)

Temporarily removing this from my sr queue, since it appears that there is a
possible alternative patch.  Conrad, once you've chosen which patch you want
and reviewed it, go ahead and add the appropriate patch to my sr list.
Attachment #126008 - Flags: superreview?(dmose)
Had to widen the wizard by 2em and reduce spacer between buttons.
Attachment #126008 - Attachment is obsolete: true
QA Contact: ktrina → gbush
Comment on attachment 126291 [details] [diff] [review]
variation of 126008 which allows OSX buttons to fit

Question: what happens when l10n makes the labels longer?
> Question: what happens when l10n makes the labels longer?

More of them will fit than currently do ;-)

To really fix this, we need a different layout for this dialog. 
We (localization people) will like any layout better that does at least not push
the buttons out of the window when selecting a longer directory name (and the
German names on Windows are that long by default, I guess others migth as well).

The layout with the buttons below the directory display tneds to be much better
there, and I really like it. The length of the buttons themnselves is not the
big problem for now, though I think that's mainly a theme and general dialog
size problem.
I guess it would be nice to include the overflow patch of Neil as well, if it
really works as expected (make only the directory display scrollable if it does
overflow).
Comment on attachment 126008 [details] [diff] [review]
Simplified patch (does the same as attachment 126006 [details] [diff] [review] above)

Removing the r= request because ccarlen posted an updated version of this patch
to make it work with the large OSX buttons.
I knew they are larger, but did not realize they are *that much* larger...

Actually I like Neil's patch better than my own one, but yes, they could also
be combined.

Robert: yes, Neil's patch works as intended (scrollbar(s) only visible if
needed). Another advantage is that the button length does not matter at all
with that patch: with larger buttons the scrollbars just appear with shorter
directory names.
Attachment #126008 - Flags: review?(ccarlen)
Comment on attachment 126291 [details] [diff] [review]
variation of 126008 which allows OSX buttons to fit

Until we know that the horizontal row of buttons actually fits in other
language other than english, lets not do this.
Attachment #126291 - Flags: review-
Comment on attachment 126021 [details] [diff] [review]
Alternatively, just make the overflow work

I think this is safer.
Attachment #126021 - Flags: review+
Conrad, so... are you going to seek sr= on this? The fix is so simple but
important, maybe it can still get into 1.4 final...
Attachment #126021 - Flags: superreview?(jaggernaut)
Comment on attachment 126021 [details] [diff] [review]
Alternatively, just make the overflow work

sr=jag
Attachment #126021 - Flags: superreview?(jaggernaut)
Attachment #126021 - Flags: superreview+
Attachment #126021 - Flags: approval1.4?
Thanks for the patch - checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Attachment #126021 - Flags: approval1.4?
verified all platforms, trunk builds 06/26
Status: RESOLVED → VERIFIED
Comment on attachment 126021 [details] [diff] [review]
Alternatively, just make the overflow work

Requesting approval for later releases on the 1.4 branch. Very simple,
virtually risk-free patch with big benefit.

jag, you removed approval1.4? after setting it (when it was clear this patch
had no chance to get in). 
So if you disagree with setting this new flag, please remove it; it is meant to
do what you might have wanted to do yourself...
Attachment #126021 - Flags: approval1.4.x?
No longer blocks: patchmaker
mozilla1.4 shipped. unsetting blocking1.4 request.
Flags: blocking1.4?
Andreas: I think I removed the 1.4 flag 'coz it was too late for 1.4 and I was
in a cleaning mood. 1.4.x seems fine, in fact I just got a request to land it on
that branch.
Keywords: fixed1.4.1
Attachment #126021 - Flags: approval1.4.x? → approval1.4.x+
Was this checked into 1.4 or not?

It has the fixed1.4.1 flag but I just approved it today.
Flags: blocking1.4.x+
Looks like jag checked it in on the 7th.
Blocks: 224532
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: