Summary says it all, need a UI to control the minimum font size preference. This is probably one of the most useful features Opera has over us right now, and it's important to get it in ASAP. All it needs it a checkbox to turn on a pull-down selection of 4,6,8,10,12pt values. Anything more should be considered an enhancement and implemented after the initial functionalty. Let's not spend months on this, *please*.

-> jbetak

cc'ing mpt for a UI proposal

Samir, as previously discussed - I'm leaving Netscape, so this might be better off with someone else. I'm cc'ing Gerv and I already pinged mpt for a UI review of this request. Last thing I've heard, there was a new font prefs dialog in the making and perhaps this functionality is already included there.

OK, duping against bug 61883, the current UI proposal seems to have what James is asking for :-) Apologies for all the spam. Fonts :::::::::::::::::::::::::::::::::::::::::::: +-Fon_ts for: [default choices :^]---------+ | _Variable With Font: [Georgia :^][ 16:^] | | Fi_xed With Font: [Courrier New :^][ 13:^] | | | | _Serif: [Times New Rom:^][Auto:^] | | Sa_ns-serif: [Trebuchet :^][Auto:^] | | _Cursive: [Zapf Chancery:^][Auto:^] | | _Fantasy: [Desdemona :^][Auto:^] | | _Monospace: [Courier New :^][Auto:^] | | | | [ ] Minimum font si_ze: [ 8:^] | +------------------------------------------------+ [/] Allow documents to use _other fonts *** This bug has been marked as a duplicate of 61883 ***

Gerv says bug 68113 has fallen off his radar and we should un-dup this one so it doesn't have to wait for all the issues in that long bug to be resolved. Reopening.

Need new owner since jbetak's contract ran out (comment #3). The checkbox [ ] that controls whether the pref is enabled or disabled might need new bool prefs per language, or perphaps, to reduce the ever extending long list of prefs, the selected value can be set to negative to instruct the back-end to ignore, while the front-end will tweak its display to set the check mark in this case.

taking

Comment on attachment 74649 [details] [diff] [review] patch + var minSizePrefVar = "font.min-size.variable." + language; + var minSizePrefFixed = "font.min-size.fixed." + language; There is only on min-size pref that governs everybody (fixed/monospace, serif, sans-serif, fantasy, cursive), and it is: + var minSizePref = "font.minimum-size." + language; So you can further simplify @@ -300,6 +306,20 @@

Note that if the value is persisted and the pref is unchecked, the back-end will still continue to read it because the back-end doesn't know if it is unchecked or not. If you want to persist the value (which is nice), you can set it to negative to cause the back-end to ignore the pref. Then simply revert the sign at display time (see comment #6).

Oh, great, I didn't know about that pref. (I now looked a bit through LXR... font.minimum-size is used in nsPresContext, while .min-size is used in gfx/src/). So I should simply ignore the font.min-size settings? >Note that if the value is persisted and the pref is unchecked, the back-end will >still continue to read it because the back-end doesn't know if it is unchecked >or not. Huh? When switching panels, I write 0 into the language data, see this line and following: + if (document.getElementById("minSizeActive").checked) And if it's 0, it will be marked as checked later. Also, if it is 0 I remove the prefs, so that the backend needs not know if it was checked. But indeed, the last selected value will remain in the listbox. I didn't think that fixing it was worth the effort.

>And if it's 0, it will be marked as checked later. Also, if it is 0 I remove >the prefs, so that the backend needs not know if it was checked. From my observations, I am anticipating some complaints if the value isn't persisted... What about saving the old value (the negative of it, that is) and extend what you are doing to allow the user to check/uncheck while keeping the same min-size value. Instead of (watch the indentation, BTW): + if (document.getElementById("minSizeActive").checked) + languageData[currentLanguage].minSize = parseInt( minSize.value ); + else + languageData[currentLanguage].minSize = 0; } have: if (document.getElementById("minSizeActive").checked) { minSizeVal = parseInt( minSize.value ); if (minSizeVal < 0) minSizeVal = -minSizeVal; languageData[currentLanguage].minSize = minSizeVal; } else languageData[currentLanguage].minSize = 0; instead of: + if (languageData[languageList.value].minSize) { have: if (languageData[languageList.value].minSize < 0) { [you get the idea]

I tried a build with the latest patch. Here are some reflections: 1) The minimum font size is correctly taken into account as soon as you hit OK in the Preferences dialog. That is, even the current page is changed (if it has smaller fonts as chosen). I assume that is by design? Unfortunately, closing Preferences after changing this option takes quiet long time (probably twice as long as if you don't change this setting). 2) If you have previously checked this option, and chosen a font size, and you later go into Preferences and uncheck this dialog, then the current page does NOT obey this setting withot a reload (like it is if you go the other way around). 3) As rbs already pointed out: it always jumps pack to size 6 if you uncheck this setting and close Preferences. Apart of these things it's fine. Thanks for doing this work Christian!

Note that if handling the negative value is troublesome, feel free to just have another pref, e.g., . font.minimum-size.<language>.selection = user's choice . if (is checked) font.minimum-size.<language> = font.minimum-size.<language>.selection else font.minimum-size.<language> = 0

Is it OK if I persist the chosen value while the dialog is open, but use 6 again when it is closed and opened again? I'd like to avoid more prefs.

better this way?

>Is it OK if I persist the chosen value while the dialog is open, but use 6 >again when it is closed and opened again? That won't be good enough for the demanding mozilla's users. The alternatives are to either use the negative trick or an additional pref. I am not a fan of too much prefs either, but an additional pref appears as the simplest implementation for you (and will save you from the trouble mentioned in point 3 in comment #14). And not all users visit the Fonts preference dialog, so...

re: "closing Preferences after changing this option takes quiet long time (probably twice as long as if you don't change this setting)." The overhead comes from the extra time to re-laid out the windows/tabs. But since the minimum-size may seem to not have an apparent effect (i.e., documents have no tiny text in the visible area), it would give the impression that closing the pref dialog takes too long. If there were a visual clue (e.g., a message on the status bar -- "applying your preferences..."), that would have helped to dilute the problem.

>The overhead comes from the extra time to re-laid out the windows/tabs. But >since the minimum-size may seem to not have an apparent effect (i.e., documents >have no tiny text in the visible area), it would give the impression that >closing the pref dialog takes too long. That's sort of what I thought, but I assumed it should only take longer if the current page actually had to be changed. Should it really take equally long to close Preferences on a page that doesn't have small fonts as on a page that has? That's what I reacted over here, but it's nothing dramatic.

Since it is not known if the windows/tabs have embedded elements with tiny fonts or no, no chance is taken, they are just relaid out.

Ok. We can't wait until the next page visit for the change to take effect?

Possibly, but the other font prefs take effect immediately, and having the minimum-size do something different might be ambiguous to users. [A general visual cue for prefs changes to indicate that something is going on seems the most attractive option to me.]

rbs, you're hard to please :) I'll work on that this weekend, won't have time before that.

Christian, I personally think your patch definately is good enough to get checked in. I just pointed out the performance issue because I know that Preferences performance is measured by the performance team, and they might notice this change. rbs, do I understand it correctly that you want the "visual cue" to be more general and not only apply here? That's probably a good idea, but not something that this patch has to handle IMHO.

general -- not meant for this bug. That's why I put as side comments in brackets. But of course, if cbiesinger comes up with a patch for that, I won't stop him :-) The thing that I think worthwhile for this bug is to persist the value as well. My guess is that somebody will file a bug about that if it isn't done. So rather than playing ping-pong with bugs that never end, it looks to me that it is worth having it right now.

ok, here is the version with the change you wanted

oh, and if the indentation is wrong in the patch, it's probably because of tabs, I will convert them to spaces.

please attach a screenshot

This is a screenshot of how the font panel looks with my patch

mpt: the apparently missing accesskey is due to bug 68841

Judging by the screenshot in attachment 75821 [details] it doesn't look like the text is aligned correctly with the listbox. Small detail, but still...

Why not remove the checkbox and have a "None" option in the dropdown? Gerv

That sounds like a good idea, I haven't thought of that myself. mpt, your opinion?

> Why not remove the checkbox and have a "None" option in the dropdown? Nice suggestion. Plus it will make the code simpler and solve the uncertainty of choosing the default option to go with the checkbox... [i.e., to activate for the first time, it will now take one click instead two].

this is a screenshot of the not-yet-attached patch

Comment on attachment 75854 [details] [diff] [review] latest patch, with gerv's suggestion r=doron

+ minSize.value = minSizeVal; + minSizeSelect( minSizeVal ); When you set selectedItem, minSize's value will be updated too, and with the adjusted value.

ah, ok, I'll remove that code, then. But why is this code in the file, then: variableSize.value = sizeVarVal; variableSize.selectedItem = variableSize.getElementsByAttribute( "value", sizeVarVal )[0]; fixedSize.value = sizeFixedVal; fixedSize.selectedItem = fixedSize.getElementsByAttribute( "value", sizeFixedVal )[0]; The .value setting could be removed, right?

ok, testing showed that it _can_ be removed, so I did. jag, could you super-review?

Comment on attachment 75854 [details] [diff] [review] latest patch, with gerv's suggestion sr=jag

Comment on attachment 75854 [details] [diff] [review] latest patch, with gerv's suggestion a=asa (on behalf of drivers) for checkin to the 1.0 trunk

checked in

Is there any special reason why font sizes 11, 13 and 15 are not available on the list? I think 13 is a nice size, but I still have to manually edit the pref.js if I want that size now.

andré, I couldn't put every size there. the ones that are there were chosen by mpt, the UI Expert. Feel free to add a new size yourself, just add a line like this to pref-fonts.xul: <menuitem value="13" label="13"/> near the others

One could argue that 11 and 13 are more useful values than 6 and 7, which are currently on the list.

It's wonderful to see this pref finally exposed! But: the pref isn't usable for me because it doesn't let me pick my chosen size of 13; 12 is too small, but if I pick 14, then I can't see my chosen monospace font any more. All the other font dropdowns show all available sizes; it makes no sense for the minimum font size dropdown to give fewer options than the other dropdowns, and it especially makes no sense for the minimum font size dropdown not to include sizes that are currently chosen for browser fonts, which just spells frustration for the user. Currently I see 21 size options in the proportional dropdown, 21 in the monospace dropdown, and 9 in the minimum size dropdown. Clearly we don't have anything limiting us to 9 items since the other two are more than double that. (Incidentally, is 16, the biggest size it offers, big enough for all low-vision users? I have no idea, but I hope someone knowledgeable about that is lurking here. Perhaps Aaron knows (cc'ing). Reopening (sorry).

(also marking attachment 75854 [details] [diff] [review] as obsolete since it has been checked in) mpt suggested adding 11, 13 and 15 so I'm doing that in this patch. also removing the minSizeSelect(size - 1) part because now every size between 6 and 16 is in the list.

Comment on attachment 76958 [details] [diff] [review] patch to add 11,13,15 r=walk84

Comment on attachment 76958 [details] [diff] [review] patch to add 11,13,15 sr=jag

Comment on attachment 76958 [details] [diff] [review] patch to add 11,13,15 a=asa (on behalf of drivers) for checkin to the 1.0 trunk

ok, I also added 18,20,22,24 on aaron's request (got r=aaronl for that) and checked that in, assuming that sr and approval were still valid. I did, of course, then not remove the minSizeSelect(size-1) line since the change made it necessary again.

Very good! Except that my default font size is 28 ! I on a 15" CRT Monitor at 1024*768, 96dpi, large font size, WinMe. And my eyes are young. For older people like my mother who has, like most people her age, problems to see clear at close range, bigger font sizes should be available. Just let all the font sizes available in the minimum font size box, and everyone will be very happy! Thanks for that great fix!

"all font sizes"? there's an infinite amount of possible font sizes!

I think it would be great if the current version was checked in. IMO, the current version will help lots of people (like me for example) as is, and the discussion of what font sizes should (not) go into the patch would be better off in another (brand new) bug. If you open such a bug, do mention it here so people can follow you over to it.

Johan: This bug is marked "RESOLVED FIXED". This means that it _is_ already checked in (now offering all sizes from 6-16 + 18,20,22,24). It's available in nightly builds starting with 20020402 (the earlier version with not so much choices is in since 20020326)

letting users enter an arbitrary size would seem like the obvious solution to the "infinite number of sizes" problem... VERIFIED that users can't set any arbitrary size but that a limited selection of sizes is available.