fix for too narrow custumizetoolbar window

RESOLVED FIXED

Status

Mozilla Localizations
nl / Dutch
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: mad_maks, Unassigned)

Tracking

({verified1.8.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

12 years ago
In Fx 2 RC1 (candidate) is the window to small. it would be good to apply this low risk patch before RC2
(Reporter)

Comment 1

12 years ago
Created attachment 239772 [details] [diff] [review]
customizetoolbar.patch

the patch
Attachment #239772 - Flags: approval-l10n?

Comment 2

12 years ago
Comment on attachment 239772 [details] [diff] [review]
customizetoolbar.patch

[nl] customizeToolbar size change approved for RC2.
Attachment #239772 - Flags: approval-l10n? → approval1.8.1+
(Reporter)

Comment 3

12 years ago
thanks, fixed in de branch (and trunk).
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 4

12 years ago
Created attachment 239901 [details] [diff] [review]
Nicer values for ctb

Tim Maks,

I took a closer look on the new dimensions caused by your patch but the pane does not really look nice IMO. The current view is much wider than needed (as you said); it causes spaces in between the icons, and a vertical scrollbar as well as 1 icon in the 4th row are still visible. If the goal was to have a pane that shows all (default, 13) icons without a vertical scrollbar, a horizontal value of 635 (good enough to see the full long text line and 4 columns) and a vertical one of 480 would suffice. These values appear to be good enough for TB's defaults as well.

Note that, at least on Windows, the absolute minimum for the pane itself appears to be around 630/490 pixels, requiring values of 621/463 (as +9/+27 is added for its real size), so 635/480 for these should suffice. Would you try them on linux and approve the patch if they are ok to you?
Attachment #239901 - Flags: review?(bugzilla)
(Reporter)

Comment 5

12 years ago
Comment on attachment 239901 [details] [diff] [review]
Nicer values for ctb

this width is needed for linux, that's why i made it so big.
Attachment #239901 - Flags: review?(bugzilla) → review-

Comment 6

12 years ago
Created attachment 239979 [details]
Screenshot on Windows

Impact on Windows.

(In reply to comment #5)
> (From update of attachment 239901 [details] [diff] [review] [edit])
> this width is needed for linux, that's why i made it so big.

I can't believe this is necessary and would very much like to see how it looks and what improvement this makes. Could you post a screenshot of what's wrong with the old, or my values?

Updated

12 years ago
Attachment #239901 - Attachment is patch: false
Attachment #239901 - Attachment mime type: text/plain → image/jpeg

Updated

12 years ago
Attachment #239901 - Attachment is patch: true
Attachment #239901 - Attachment mime type: image/jpeg → text/plain

Updated

12 years ago
Attachment #239979 - Attachment is patch: false
Attachment #239979 - Attachment mime type: text/plain → image/jpeg
(Reporter)

Comment 7

12 years ago
Created attachment 240112 [details]
screenshot linux

In my opinion is it better to have a big screen on windows (people can make it smaller if the want and then it stay that size) then to small on a other platform where people can't see all the buttons.

Comment 8

12 years ago
It looks like the DPI setting on your Linux machine is higher...

If you set the Windows DPI setting higher, you’ll probably see the same result? Is your DPI setting on Linux the default setting, or did you modify it (e.g. because the screen is high-res)?


~Grauw

Comment 9

12 years ago
Created attachment 240214 [details]
Screenhot (Windows large fonts, patched)

(In reply to comment #8)
> It looks like the DPI setting on your Linux machine is higher...

That's probably what I suspected, Tim must be using (very) large fonts. (?)

Indeed at 120dpi (large fonts) on Windows a similar thing happens using the old values, but not as bad as the linux screenshot, so even then the new values make it wider than needed (see screenshot).

Therefore I'd suggest backing out the patch and not to change the old values into any other, for several reasons:

- only a limited number of users are 'served' by it
- many more will be bugged by it because of the huge width (most users on several platforms) and may want to resize
- it's just a first-time issue; the pane size is resizable anyway
- the pane can be set to full screen by clicking its bar, which I presume most users would do
- nl would be the only locale doing this (see http://lxr.mozilla.org/l10n-mozilla1.8/search?string=dialog.style)
- if such a change would be considered useful, it might be better to file a bug for en-US first, with or without a proposal for 'an auto-resize-pane-width' for large fonts
(Reporter)

Comment 10

12 years ago
this is a standard Linspire install. i will check this later this week on a other system.

but i do not see a reason to back this patch out. the pane size is resizable, so people who think it is toi big will make it smaller.

Comment 11

12 years ago
(In reply to comment #10)
> this is a standard Linspire install. i will check this later this week on a
> other system.

That's too late for FF2 ;-(

> but i do not see a reason to back this patch out. the pane size is resizable,
> so people who think it is toi big will make it smaller.

Many reasons are listed up above. Only a few will experience the problem you have (for which Linspire is probably to blame), so the impact is just too big. Besides, we're responsibe for proper l10n, not for differences in program behaviour. Therefore I consider this to be a mess-up.
(Reporter)

Comment 12

12 years ago
(In reply to comment #11)

> Many reasons are listed up above. Only a few will experience the problem you
> have (for which Linspire is probably to blame), so the impact is just too big.
> Besides, we're responsibe for proper l10n, not for differences in program
> behaviour. Therefore I consider this to be a mess-up.
> 
i am sorry that you see this as such a big problem. your reasons can also be used the other way around. 

Comment 13

12 years ago
Could you please check if these bugs were landed? I see check-ins, but the check-in comments don't refer to bug numbers.

Please add the fixed1.8.1 keyword if so, and change that to verified1.8.1 if you have VERIFIED the fixes on an RC2 or 3.
(Reporter)

Comment 14

12 years ago
Mozilla/5.0 (X11; U; Linux i686; nl; rv:1.8.1) Gecko/2006100316 Firefox/2.0
Keywords: verified1.8.1
You need to log in before you can comment on or make changes to this bug.