30896 - no UI for "Use my default fonts" pref for mail / message display

Status: NEW

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[Akkana Peck]

The fonts pref panel has a pref for "Use my default fonts even if the document
specifies otherwise".

I don't set that in the browser, because it isn't a problem that often and I
want to see most html pages the way they're formatted, but in mail, all I care
about is reading the text, and I would love to be able to set this pref for mail
only in order to read messages from OE users who set their fonts too small.  We
have other prefs like "disable JS" that can be set separately for mail and
browser; any chance we could add "use default fonts" to the list?

(It would also be neat to be able to turn off showing colors and backgrounds,
but that's probably harder and not as critical.)

Comment 1

[Alec Flett]

not sure how this stuff works, adding rich to the CC
adding Jennifer so maybe she can design a UI around this. It seems like a good
idea to me

Comment 2

[rhp (gone)]

Actually, to change the mail display prefs, see the follow:

user_pref("font.name.monospace.x-unicode", "Font_Name");    --> plain text mail 
body
user_pref("font.name.serif.x-unicode", "Font_Name");   --> HTML mail body

user_pref("font.size.fixed.x-unicode", 12);   --> sizes in pixels
user_pref("font.size.variable.x-unicode", 16);  --> sizes in pixels

Comment 3

[Akkana Peck]

Rich, are you saying that setting these fonts will override tags like <font
face=""> in the message?  I assumed that the font prefs you mention only applied
to mail that does not specify a font face or size (I'm fairly sure that was how
4.x worked).

If true, then this bug is invalid, because it's already the case anyway (though
someone might want the opposite bug, requesting a way to see the message's
specified fonts).

Comment 4

[Phil Peterson]

P4 per m/n staff mtg today

Comment 5

[Alec Flett]

wondering if this should go to RHP or putterman.
Not sure if this is a "feature" bug per say. taking off [FEATURE] list for now.

Comment 6

[selmer (gone)]

Since this bug is marked P4, moving to M17.  If you disagree, please let me know.

Comment 7

[selmer (gone)]

Not critical for this release.  M20.

Comment 8

[Alec Flett]

give to putterman

Comment 9

[Ben Bucksch (:BenB)]

I fixed this months ago with bug 18427.

*** This bug has been marked as a duplicate of 18427 ***

Comment 10

[(not reading, please use [e-mail] instead)]

where is the UI for this?

re-opening

Comment 11

[(not reading, please use [e-mail] instead)]

taking from putterman.

Comment 12

[Ben Bucksch (:BenB)]

> where is the UI for this?

nowhere, but the reporter didn't ask for it :). I'd like to have UI as well,
shouldn't be hard. Ask me for some hints, when you are about to implement it.

> (It would also be neat to be able to turn off showing colors and backgrounds,
> but that's probably harder and not as critical.)

BTW: That works, too.

Comment 13

[Anthony Graham]

*** Bug 107885 has been marked as a duplicate of this bug. ***

Comment 14

[(not reading, please use [e-mail] instead)]

ssu is going to take this bug.

here's where he'll start:

start by figuring out where the browser fonts affect browser and mail.
nsPresContext::GetFontPreferences() looks very likely

if that's correct, see if there is a way to distinguish between a browser
presentation context and our message pane presentation context.

my guess is there will be (some how, might require some investigation)
we can tweak that code so that we ask for the mail prefs when it's a mail
presentation context.

if that works out, we would then make sure that the pref observer code (the code
that gets notified when the font pref values change) does the right thing in
mail when the mail fonts change.

then we need to find a way to share the pref font UI (pref-fonts.xul and
pref-fonts.js) between both mail and browser.  we should be able to re-use and
share it.

Comment 15

[Sean Su]

mine now.

Comment 16

[scottputterman]

Jennifer, any UI ideas for this?

Comment 17

[jglick]

I take it we want a separate ui in mail Prefs to display these settings? I'll 
take a look.

Comment 18

[jglick]

Attachment: Example

See HTML Messages group box. Is this what we want? Let me know nec changes.

Comment 19

[Ben Bucksch (:BenB)]

Would be a nice candidate for bug 31907.

Comment 20

[Sean Su]

jglick, the current mailnews Message Display pref page has a "Languages"
section.  Where should that be moved to in order to fit the new "HTML Messagees"
section?

Comment 21

[Sean Su]

jglick: nevermind.  I just saw the "Languages" pref panel section in your image :)

Comment 22

[jglick]

http://www.mozilla.org/mailnews/specs/prefs/Preferences.html#Lang
:-)

Comment 23

[Sean Su]

Attachment: patch v0.1

This patch is no where near done, but it provides clues to where changes might
take place.

It can already detect if the browser element is in the mailnews context or not.


I'm still working on figuring out where to do the checks for if the browser is
in the mailnews context.

Comment 24

[(not reading, please use [e-mail] instead)]

the plan is to do this with prefs, not userChrome.css (see ssu's start of a patch)

qa -> laurel (or stephend).  no more rhp.

Comment 25

[(not reading, please use [e-mail] instead)]

Currently, the code in nsPresContext does this:

1) in ::Init() register pref callbacks for "font."
2) in ::Init() call GetUserPreferences() [which calls GetFontPreferences()]
3) in ::~nsPresContext(), unregister the pref callback for "font."

Here's what ssu is proposing to do:

1) move the pref callback registration to ::SetShell()
2) in ::SetShell(), determine if the browser element has a fontprefix
attribute(*), if so, store the fontprefix in a nsCString member variable mFontPrefix
3) in ::SetShell(), register a pref callback for mFontPrefix + "." + "font."(if
mFontPrefix is not empty, otherwise just "font.")
4) in ::SetShell(), call GetUserPreferences(), heeding the mFontPrefix member
variable.
5) in ::~nsPresContext(), unregister the pref callback for mFontPrefix + "." +
"font."(if mFontPrefix is not empty, otherwise just "font.")

Here's a code snippet of how ssu plan's on determining the font prefix:

+  if (mShell) {
+    nsCOMPtr<nsIDocument> doc, parentDoc;
+    mShell->GetDocument(getter_AddRefs(doc));
+    doc->GetParentDocument(getter_AddRefs(parentDoc));
+    if (parentDoc) {
+      // get content for <browser>
+      nsCOMPtr<nsIContent> browserElContent;
+      parentDoc->FindContentForSubDocument(doc, getter_AddRefs(browserElContent));
+      nsCOMPtr<nsIDOMElement> browserElement = do_QueryInterface(browserElContent);
+
+      if (browserElement) {
+        browserElement->GetAttribute(NS_LITERAL_STRING("fontprefix"), mFontPrefix);

layout should not have any hard coded knowledge of mail.

Comment 26

[Simon Fraser [no longer active]]

+nsPresContext::IsMailnewsContext()

This leaves a nasty taste in my mouth.

Comment 27

[(not reading, please use [e-mail] instead)]

The part that sfraser objects to is going away.  I didn't cc sfraser on my
private mail, so let me post it here for comments:

Just like the user can set default font prefs for the browser, we want to allow
the user to be able to set default prefs for message display.

But we don't want layout to have any hard coded knowledge of mail.

Currently, the code in nsPresContext does this:

1) in ::Init() register pref callbacks for "font."
2) in ::Init() call GetUserPreferences() [which calls GetFontPreferences()]
3) in ::~nsPresContext(), unregister the pref callback for "font."

Here's what ssu is proposing to do:

1) move the pref callback registration to ::SetShell()
2) in ::SetShell(), determine if the browser element has a fontprefix
attribute(*), if so, store the fontprefix in a nsCString member variable mFontPrefix
3) in ::SetShell(), register a pref callback for mFontPrefix + "." + "font."(if
mFontPrefix is not empty, otherwise just "font.")
4) in ::SetShell(), call GetUserPreferences(), heeding the mFontPrefix member
variable.
5) in ::~nsPresContext(), unregister the pref callback for mFontPrefix + "." +
"font."(if mFontPrefix is not empty, otherwise just "font.")

Does this sound reasonable?

Here's a code snippet of how ssu plans to determine the font prefix:

+  if (mShell) {
+    nsCOMPtr<nsIDocument> doc, parentDoc;
+    mShell->GetDocument(getter_AddRefs(doc));
+    doc->GetParentDocument(getter_AddRefs(parentDoc));
+    if (parentDoc) {
+      // get content for <browser>
+      nsCOMPtr<nsIContent> browserElContent;
+      parentDoc->FindContentForSubDocument(doc, getter_AddRefs(browserElContent));
+      nsCOMPtr<nsIDOMElement> browserElement = do_QueryInterface(browserElContent);
+
+      if (browserElement) {
+        browserElement->GetAttribute(NS_LITERAL_STRING("fontprefix"), mFontPrefix);

Comment 28

[(not reading, please use [e-mail] instead)]

Comment on attachment 101470 [details] [diff] [review]
patch v0.1

marking as obsolete / needs work, to make sure people know this isn't what we
plan to check in.

Comment 29

[Jo Hermans]

*** Bug 180351 has been marked as a duplicate of this bug. ***

Comment 30

[(not reading, please use [e-mail] instead)]

taking

Comment 31

[esther]

per seth, I'm going to mark target milestone as future

Comment 32

[Felix Miata]

Didn't the bug 30888 fix make this functionally obsolete?

Comment 33

[Ben Bucksch (:BenB)]

*** Bug 263018 has been marked as a duplicate of this bug. ***

Comment 34

[entonio]

Please look at Bug 351982 which I think best captures the problem with this and bug Bug 178362.