353928 - fix for too narrow custumizetoolbar window

Status: RESOLVED FIXED

(Mozilla Localizations :: nl / Dutch, defect)

Description

[Tim Maks van den Broek [:mad_maks]]

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

Comment 1

[Tim Maks van den Broek [:mad_maks]]

Attachment: customizetoolbar.patch

the patch

Comment 2

[Axel Hecht [:Pike]]

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

[nl] customizeToolbar size change approved for RC2.

Comment 3

[Tim Maks van den Broek [:mad_maks]]

thanks, fixed in de branch (and trunk).

Comment 4

[Ton [:Tonnes]]

Attachment: 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?

Comment 5

[Tim Maks van den Broek [:mad_maks]]

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.

Comment 6

[Ton [:Tonnes]]

Attachment: 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?

Comment 7

[Tim Maks van den Broek [:mad_maks]]

Attachment: 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

[u81239]

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

[Ton [:Tonnes]]

Attachment: 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

Comment 10

[Tim Maks van den Broek [:mad_maks]]

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

[Ton [:Tonnes]]

(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.

Comment 12

[Tim Maks van den Broek [:mad_maks]]

(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

[Axel Hecht [:Pike]]

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.

Comment 14

[Tim Maks van den Broek [:mad_maks]]

Mozilla/5.0 (X11; U; Linux i686; nl; rv:1.8.1) Gecko/2006100316 Firefox/2.0