Closed Bug 123370 Opened 23 years ago Closed 22 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)

(deleted), image/png
Details
(deleted), patch
timeless
: review+
dmosedale
: superreview-
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), patch
ccarlen
: review+
jag+mozilla
: superreview+
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), 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) (deleted) — 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. ***
Attached patch Updated for review (deleted) — Splinter Review
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) (deleted) — 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) (deleted) —
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) (deleted) — 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) (deleted) —
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?
Attached patch Buttons horizontally, grouped (deleted) — Splinter Review
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,...).
Attached image Screenshot of last patch (deleted) —
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: 22 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: