use firefox default font chooser for display

RESOLVED FIXED in Thunderbird 3.0b4

Status

Thunderbird
Preferences
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: clarkbw, Assigned: bwinton)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Thunderbird 3.0b4
Dependency tree / graph
Bug Flags:
blocking-thunderbird3 -
blocking-thunderbird3.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [has l10n impact], URL)

Attachments

(4 attachments, 7 obsolete attachments)

(Reporter)

Description

10 years ago
Created attachment 335984 [details] [diff] [review]
port of firefox font display

Our current interface for changing the fonts in the Display preferences is scary and hidden.  An easy way to catch up to modern times would be to take what Firefox has done and begin using that for our font preferences.

This means that the default font option gets moved to a menulist on the main tab pane with a sizing option beside it.  The original font dialog is now launched from the (Advanced...) button.  The font related plain text items have been moved into the advanced dialog.

This is basically a straight port of the firefox code over to Thunderbird.  I didn't reuse the HTML font chooser from the Compose pane because it is quite awkward and likely could use to be converted to this system.
(Reporter)

Comment 1

10 years ago
Created attachment 335985 [details]
screenshot of the font changes

Here's an after shot of the changes to the display preferences and the font dialog

Comment 2

10 years ago
Yes, this would be very helpful. Many users are trying to modify the font-size preferences in the Composition pane instead, as they don't find the "real" setting they need to find. Putting this upfront in the Display pane with an "Advanced" button would be beneficial for the novice users.

If this does provide an interface for the proportional font sizes only, should the monospace font be presented in a similar way, or can those two sizes be somehow connected? Messages or paragraphs received in preformatted style may be unaffected by the size selection otherwise.

Comment 3

10 years ago
rsx11m: that's why there is the "allow messages to use other fonts" option. These settings just affect fonts in general. For html mails the fonts they have specified are used, if none are specified, the defaults are used (I think).


How about getting rid of the radio buttons to make it
 "Use a fixed width font for plain text messages [ x ]"
OS: Linux → All
Hardware: PC → All

Comment 4

10 years ago
I just did some testing based on Magnus' comment. The "allow messages to use other fonts" option does not change the distinction between variable and fixed fonts. Everything explicitly sent with "variable" font uses the pixel size in the upper box, whereas everything with "fixed" font (or sent with "preformat" style, also replies to non-flowed plain-text messages) uses the size specified in the lower box (you can set them to extreme values, e.g., 8 vs. 24 to see which one is used for a given text). Thus, it would be good to have both of those sizes visible in the main pane, perhaps

   Default font: [ sans-serif    v ]  [ Advanced... ]
   Size (pixels): Proportional [15v], Monospace [12v]
(Reporter)

Comment 5

10 years ago
>    Default font: [ sans-serif    v ]  [ Advanced... ]
>    Size (pixels): Proportional [15v], Monospace [12v]

That's a pretty good change, but I'd still rather have a really simple font selector for people and avoid the mono vs. proportional confusion.  Though for the font control to work uniformly we'd have to default to a proportional font everywhere.

> How about getting rid of the radio buttons to make it
>  "Use a fixed width font for plain text messages [ x ]"

Yeah that would clean up that piece a lot.  I'll give it a try.

*Font Control*
[x] Allow messages to use other fonts
[ ] Use fixed width font for plain text messages

- Though I wonder if I should be using "Monospaced font" in this section as "Proportional" and "Monospaced" are both there in the section above.  I don't really see "fixed width" as more user friendly in this context.
(Reporter)

Comment 6

10 years ago
Created attachment 339878 [details] [diff] [review]
port of firefox font display v2

Here's an updated version of the patch with the changes described.  Screenshot to follow
Assignee: nobody → clarkbw
Attachment #335984 - Attachment is obsolete: true
(Reporter)

Comment 7

10 years ago
Created attachment 339879 [details]
screenshot of the font changes v2

here's the screenshot of the updated patch
Attachment #335985 - Attachment is obsolete: true
(Reporter)

Comment 8

10 years ago
also, I just found bug 323517 which highlights the terminology issues of fixed/variable and proportional/monospaced
(Reporter)

Comment 9

10 years ago
Just wanted to make a clear note on why the default preference change for mail.fixed_width_messages

Proportional and Monospaced fonts sizes can't be set to the same sizes with good results, monospace fonts will often look much larger.  So there isn't a way to have a single font size selector that changes all the fonts used in messages display.  However we need a single font size selector for the default case.

By defaulting to using proportional fonts for all messages being displayed we can have a single font sizer that changes all the displayed fonts.  In a default scenario a person could receive a plain text message or an html message and use the display font preference to change the font size of both messages to a larger or smaller value.

In the use case where a person has set monospaced fonts for plain text messages they can then continue using the main interface for changing proportional font sizes and the advanced font popup for changing either of the font sizes.

Our users require a single place to change all font sizes.  For the people who don't understand or don't care about the difference between monospaced and proportional fonts we need to have a way to change the font size.  We could try to educate users about the difference in fonts however this kind of user education often returns very limited rates of success.  If we have the two font size selectors users are likely to set the font sizes to the same larger value; this will render some messages very large and some messages smaller.  And the visual difference between the plain text message and the html message is often not distinguishable.

It takes 5 total clicks to change this preference back to the current default -> Edit/Tools, Preferences, Display, (Fonts) Advanced, and [x] ...Plain Text.  However I think there are several things we can do to make this preference irrelevant in the longer term.

* bugzilla mail expects monospaced fonts
** bug mail can be easily detected and should likely default to a monospace font
** this would require some of the planned changes to the filtering system
* newsgroups should likely default to monospace fonts
** this will probably require a pref for newsgroups

It might be good to open bugs for some of these cases as this change moves forward.
(Reporter)

Updated

10 years ago
Blocks: 456587

Comment 10

10 years ago
Somewhat unrelated, but a pet peeve of mine (and for corporate users):

When "Tools / Options / Composition / HTML / Font = ARIAL" is chosen, and the user does any of the following:

- creates bullet/numbered list
- interspersed quoting
- other?

Then the following text will change to Times New Roman (serif).

This makes e-mails not look the way the user intended, not to mention unprofessional and a conglomeration of sans-serif and serif fonts.

Comment 11

10 years ago
I looked at the V2 screenshot and think it's a good improvement over the current dialog. One reason I say that is the placement of the Advance button. It is clear there are more personalization possibilities. 

How does this relate to the Compose tab for selection of Write window default fonts?  Are You using another bug for rework of the compose tab?
(Reporter)

Comment 12

10 years ago
I don't really like the compose tab font chooser because it's asking the person to help select a default font for writing mail.  I'd much rather we just remember the last font used for writing mail and keep that as the default font for composing mail.

There are obvious consequences of this kind of remember the last choice default but having a preference for defaults rarely actually resolves those consequences, it just moves it around.  People with real font-changing needs would be helped more by a good template system.

So I guess my composer tab font bug would be a separate bug: 'Remove compose font preferences, remember fonts used in composer'

Comment 13

10 years ago
When You open such bug would You put Me on the CC list please.
Created attachment 378719 [details] [diff] [review]
udpated patch

I'm pretty sure this is ready for review but I don't want to slow down the reviewers with this.  I'll target this bug for b4 so I remember to get it in by then.
Attachment #339878 - Attachment is obsolete: true
(Reporter)

Updated

9 years ago
Flags: blocking-thunderbird3+
Whiteboard: [has patch][needs review]
Target Milestone: --- → Thunderbird 3.0b4
Bryan, can you request review now so we can get this bug fixed?

Comment 16

9 years ago
Before this goes final for review, one minor tweak suggestion.  Since the length of the strings sans-serif, etc. are shorter than the width of the picker, narrow the pickers width. You will then have less congestion in that row to accommodate the Advance button.
(In reply to comment #16)
> Before this goes final for review, one minor tweak suggestion.  Since the
> length of the strings sans-serif, etc. are shorter than the width of the
> picker, narrow the pickers width. You will then have less congestion in that
> row to accommodate the Advance button.

I think that menulist is given a flex=1

I'll try removing that and see what it looks like.

Updated

9 years ago
Depends on: 507795

Updated

9 years ago
Whiteboard: [has patch][needs review] → [needs updated patch][needs review]
hoping blake can whip through this.
Assignee: clarkbw → bwinton
(Assignee)

Comment 19

9 years ago
Created attachment 393700 [details]
The menulist with flex="1" removed.

(In reply to comment #17)
> > Before this goes final for review, one minor tweak suggestion.  Since the
> > length of the strings sans-serif, etc. are shorter than the width of the
> > picker, narrow the pickers width. You will then have less congestion in that
> > row to accommodate the Advance button.
> I think that menulist is given a flex=1
> I'll try removing that and see what it looks like.

It doesn't look so good, particularly on the right side.  :)

It's true that "sans-serif" is smaller than the space given, but "Bitstream Vera Sans Mono" and "Hiragino Maru Gothic ProN" both seem to be larger than the space given, so I don't think we can reduce the space given to the dropdown all that much.  I did throw in a 15px spacer before the Advanced button, to try and loosen it up a little, and I'll throw up a picture of that next.

Later,
Blake.
(Assignee)

Comment 20

9 years ago
Created attachment 393701 [details]
The dialog with the 15px spacer.
(Assignee)

Comment 21

9 years ago
Created attachment 393702 [details] [diff] [review]
The patch for the previous screenshot.  (Heavily based off of Bryan's.)

Bryan, let me know how you like this version, and I'll clean up the javascript and post the new patch for code-review.
Attachment #378719 - Attachment is obsolete: true
Attachment #393702 - Flags: ui-review?(clarkbw)

Comment 22

9 years ago
The new screen shot looks cleaner to me, befits from losing the graphic in the button and fixed the crowding I commented on.

Comment 23

9 years ago
Re the "(pixels)" label we currently have, is there a reason not to put the uniti inside the dropdown like 
[ 16 px ]
| 17 px |
| 18 px |
(Assignee)

Comment 24

9 years ago
Created attachment 394870 [details] [diff] [review]
The previous patch, cleaned up and with mkmelin's suggestions

(In reply to comment #23)
> Re the "(pixels)" label we currently have, is there a reason not to put the
> uniti inside the dropdown like 
> [ 16 px ]
> | 17 px |
> | 18 px |

Changed.

(My only potential question would be how it looked from a localization point of view.  Is "px" for pixel common enough for people who don't speak english to understand?  It's everywhere in css/html, but is that good enough?)

Later,
Blake.
Attachment #393702 - Attachment is obsolete: true
Attachment #394870 - Flags: ui-review?(clarkbw)
Attachment #394870 - Flags: review?(mkmelin+mozilla)
Attachment #393702 - Flags: ui-review?(clarkbw)
(Assignee)

Updated

9 years ago
Whiteboard: [needs updated patch][needs review] → [needs review]
I don't think that hardcoding "n px" works well.

You could do a plural-dependent localizable property instead, but asking :tb-l10n for wider input.

For Brian's ui-review, I'm not sure if the font size is really that ambiguous that it requires adding a unit to it. Didn't investigate this in-depth of where it got added and why.
(In reply to comment #25)
> For Brian's ui-review, I'm not sure if the font size is really that ambiguous
> that it requires adding a unit to it. Didn't investigate this in-depth of where
> it got added and why.

Yeah, I agree.  We could put the unit of measure in the title for the label so it shows on hover but the majority of people are looking for "bigger or smaller than currently", not 14 pixels of height exactly; which is pretty inaccurate anyway.
(Assignee)

Comment 27

9 years ago
Created attachment 394964 [details] [diff] [review]
The previous patch, without the "px"s.

(In reply to comment #26)
> (In reply to comment #25)
> > For Brian's ui-review, I'm not sure if the font size is really that
> > ambiguous that it requires adding a unit to it. Didn't investigate this
> > in-depth of where it got added and why.
> Yeah, I agree.  We could put the unit of measure in the title for the label so
> it shows on hover but the majority of people are looking for "bigger or
> smaller than currently", not 14 pixels of height exactly; which is pretty
> inaccurate anyway.

Okay, I've removed the px-s, and the (pixel)-s.  If it's not accurate, I'ld rather not put it in the title, even.

Thanks,
Blake.
Attachment #394870 - Attachment is obsolete: true
Attachment #394964 - Flags: ui-review?(clarkbw)
Attachment #394964 - Flags: review?(mkmelin+mozilla)
Attachment #394870 - Flags: ui-review?(clarkbw)
Attachment #394870 - Flags: review?(mkmelin+mozilla)
It was added (very roughly speaking) in bug 89842, because bz used a single profile on different machines with different monitor resolutions and by having it say whether it was pixels or points he would know whether he needed to choose something larger than perfect for one system so it would still be readable on another one. I'm sure there must be fives, though probably not tens, of users in that same situation and class of knowledge ;)

Updated

9 years ago
Whiteboard: [needs review] → [needs review][has l10n impact]
Comment on attachment 394964 [details] [diff] [review]
The previous patch, without the "px"s.

looks good to me.
Attachment #394964 - Flags: ui-review?(clarkbw) → ui-review+
The next part of this, which may be an additional bug, is setting the correct language according to the message being displayed.  Currently we have an issue for many users who may set the default font but it only sets for their language.  

Any message which arrives as utf-8 or "Other Languages" will use a different font.  The advanced font chooser allows a person to edit this but I think for the not so advanced person we need a better system.  It was suggested that we could look for the Character encoding of the displayed message and use that for the "default" we would be changing.  This way a person changing the font size in the prefs would always see the change being applied to the message being displayed (something that doesn't always happen otherwise).

With this change we might also want to at least show the character encoding as a label in this default view.

Comment 31

9 years ago
Reply to Comment #30

Yes, You have hit on one of the sophisticated ability's of Moz with the extensive list of hidden prefs. I ended up using Config Edit / about:config to maualy reset dozens of the font prefs and sizes.

One solution is more usage of *.otf (Open Type Format) fonts with very wide arrays of language encodings. The picking of one font can deal with display of many languages with a consistent look and feel. The limit is e-mail generators properly MIME typing the text encodings throughout a mixed encoded message.
The Thunderbird drivers wish to release Thunderbird 3 as soon as possible. As a
result, we feel that this bug shouldn't stand in the way of all the other good
work getting into the hands of users sooner rather than later. Therefore we are
retargeting it for 3.1. See http://ccgi.standard8.plus.com/blog/archives/242
for more details. The 3.1 release is expected to be a quick release soon after
Thunderbird 3.
Flags: blocking-thunderbird3.1+
Flags: blocking-thunderbird3-
Flags: blocking-thunderbird3+
Target Milestone: Thunderbird 3.0b4 → ---

Updated

9 years ago
Attachment #394964 - Flags: review?(mkmelin+mozilla) → review+

Comment 33

9 years ago
Comment on attachment 394964 [details] [diff] [review]
The previous patch, without the "px"s.

>+pref("mail.fixed_width_messages", false); // plain text messages use variable width font

Don't want this change, we'd get huge complaints. 

>-    this.mTagListBox = document.getElementById('tagList');    

Make that double quotes while you're at it ;)

>+  readRDFString: function(aDS,aRes,aProp)

While you're touching it, add spaces between the args

>+  writeFixedWidthForPlainText: function ()
>+  {
>+    var mailFixedWidthMessages = document.getElementById("mailFixedWidthMessages");
>+    return mailFixedWidthMessages.checked ? 1 : 0;
>+  },

It's a boolean pref, so return true : false
 
>-<!ENTITY  sizes.label                             "Size (pixels)">
>+<!ENTITY  size.label                              "Size:">
> <!ENTITY  proportionalSize.accesskey              "e">
> <!ENTITY  monospaceSize.accesskey                 "x">

The x doesn't work nicely anymore.

r=mkmelin with those fixed
Comment on attachment 394964 [details] [diff] [review]
The previous patch, without the "px"s.

size.label should change name if localizers are expected to pick this change up.

Comment 35

9 years ago
It did (it was sizes.label, is now size.label)
(Assignee)

Comment 36

9 years ago
Created attachment 395912 [details] [diff] [review]
The previous patch, with mkmelin's suggestions.

(In reply to comment #33)
> (From update of attachment 394964 [details] [diff] [review])
> >+pref("mail.fixed_width_messages", false); // plain text messages use variable width font
> Don't want this change, we'd get huge complaints. 

Strongly agreed!  Fixed.

> >-    this.mTagListBox = document.getElementById('tagList');    
> Make that double quotes while you're at it ;)

Fixed, and all the other ones in the file, cause I was there.

> >+  readRDFString: function(aDS,aRes,aProp)
> While you're touching it, add spaces between the args

Fixed.

> >+  writeFixedWidthForPlainText: function ()
> >+  {
> >+    var mailFixedWidthMessages = document.getElementById("mailFixedWidthMessages");
> >+    return mailFixedWidthMessages.checked ? 1 : 0;
> >+  },
> It's a boolean pref, so return true : false

Changed to "return mailFixedWidthMessages.checked;".  I also changed "return useDocumentFonts.checked;", because it followed the same pattern.

> >-<!ENTITY  sizes.label                             "Size (pixels)">
> >+<!ENTITY  size.label                              "Size:">
> > <!ENTITY  proportionalSize.accesskey              "e">
> > <!ENTITY  monospaceSize.accesskey                 "x">
> The x doesn't work nicely anymore.

I changed it to "i", cause that's the only character that wasn't taken.  Well, I guess I could have changed it to ":".  ;)

> r=mkmelin with those fixed

Thanks,
Blake.
Attachment #394964 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/1d88547281a4
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs review][has l10n impact] → [has l10n impact]
Target Milestone: --- → Thunderbird 3.0b4
You need to log in before you can comment on or make changes to this bug.