Closed Bug 690644 Opened 13 years ago Closed 6 years ago

Composition options should allow NOT choosing custom text/background colors by default

Categories

(Thunderbird :: Message Compose Window, defect)

Product:
Thunderbird
Component:
Message Compose Window
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 65.0

People

(Reporter: musiphil, Assigned: palswim+mozilla.org)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 15 obsolete files)

690644.png - screenshot of (v3d)
8.05 KB, image/png
Details
690644-default-colors.patch (v3e, JK) with note - rebased a second time
12.98 KB, patch
mkmelin
: review+
jorgk-bmo
: ui-review+
Details | Diff | Splinter Review 
Under Options > Composition > General > HTML, there are color pickers for text and background colors. Those colors are used in the BODY tag of the HTML message, as in <body text="#000000" bgcolor="#ffffff">.

When you want to use the default colors for the recipient (i.e. <body> without custom color assignments), you can do it by opening Format > Page Colors and Background in the composition window and choose "Reader's default colors (Don't set colors in page)". However, there's no way to make it the default, as the Options dialog forces you to choose colors.

The Options dialog should also allow you to leave them unspecified.

(There's a huge inconsistency between the Options dialog and the Page Colors and Background dialog; i.e. why isn't it possible to specify colors for link text, etc. in the former? But this is a separate issue.)
Component: Preferences → Message Compose Window
QA Contact: preferences → message-compose
Version: 3.1 → Trunk
OS: Windows 7 → All
Hardware: x86 → All
Agree that this is annoying :

I've gone some way to set up my client for a dark theme to reduce eyestrain. This is somewhat spoiled by emails that force their background colour to white.

So I turn off the "Allow content to choose it's own colours" option, but this is not the best solution

 - Turning this OFF reduces the visibility of inline comments in HTML emails where the replying client uses colour to identify them.

 - Turning this ON allows emails to use colours, which makes inline replies in HTML visible again.

Imagine my surprise when I realized the main reason I had to turn this option off wasn't Outlook!! Looking at the sources of some of my fellow correspondents I realized that Outlook isn't forcing "stationery" colours (except in the mail signatures, which isn't the fault of Outlook, it's the fault of some web designer). Even mail composed in Word doesn't seem to force the colours!

I can't set my composing colours to light grey on dark grey because the corporate world would have a fit over this egregious display of non-conformity. And besides, it would be rude for ME to presume that they don't need high-contrast, etc.

So : it would be a better email world if HTML compose had a way NOT to force colours on your "stationery". I could compose in peaceful grey-on-grey, and everyone else can view my mail in their chosen colour scheme without having to dive into their options dialogs, or compromise on features like the visibility of inline comments.

In an ideal world

 * Everyone would just use a standard stylesheet for HTML email and we could pick our own colours

Alas, it's not like this...

To address the scenario above... black and white are going to by far be the most popular colours for forced background / text

So maybe

 1) Give HTML compose the ability to NOT set foreground / background colours

1a) Even better if this is the default, since this seems to be how most of the other major clients implement it, and forcing the colours is just rude.

 2) To deal with the scenario where you want a dark theme but don't want to miss inline comment highlights, have a special mode that renders just black and white as the colours of your choice but leaves the rest alone (although this seems inelegant).
So, correct me if I am wrong, because maybe I have tested it in improper way:
Add-ons disabled, HTML signature disabled, default e-mail composition colors (black text/white background).

Code comment says: "If any color attribute is set in the body, mode is "Custom Colors", even if 1 or more (but not all) are actually null (= "use default") When in "Custom Colors" mode, all colors will be set on body tag, even if they are just default colors, to assure compatible colors in page. User cannot select "use default" for individual colors"

Yes, I admit, the way it is supposed to work sounds nice, but I think the code that should do the check (and which meanwhile sets custom colors from runtime variables) always returns true, even if the colors are left at their default settings?

That results in all HTML e-mails getting forced text/background colors.

NOTE: Following only applies, if my reasoning above is correct.

Therefore, as the check doesn't work anyway it can be:
1) fixed - but I'm not willing to do that, while the idea is nice - IMHO sender should care about the sanity checks himself
1a) notification similar to 'attachment missing' would appear - not volunteering myself either, but it sounds like a reasonable action to take
2) removed, and replaced with global config radiobox in order to make experience consistent, as proposed by Adrian - <b>I will provide a patch for that</b>

Anyway, if the automatic check (1) would be re-implemented, I think the global config should be extended in the same tri-state way as "override the specified color with global config display colors".
It would allow for:
0 - never include color in <body>
1 - force include color in <body>
2 - basing on the check make a decision about including the color setting in <body>

Cheers,
MK
I want to second this feature request. I'm using a dark theme myself, and apparently my own emails get sent with an enforced color scheme, and there's seemingly no way to unset that (even if I specify "inherit" for both colors in about:config, it doesn't work).
Status: UNCONFIRMED → NEW
Ever confirmed: true
I would have added this patch via MozReview, but my Mercurial software is out of date. Please review.
I too would like to bump this request. I wasn't aware TB was enforcing my (dark) composition color preferences for recipients receiving HTML email until one such recipient brought to my attention that he didn't like it. Web email clients (gmail, yahoo, etc.) strip out these colors.

I'm opting to send in plain text format (which I can set globally) until this issue is addressed with a global or account level setting for "Reader's default colors" as remembering to set it on a per message basis is simply not a practical solution for me.

Thanks,
—T
Attachment #8910866 - Flags: review?(bwinton)
Comment on attachment 8910866 [details] [diff] [review]
Added new preference to select "Reader's default colors" for any new message

It seems okay to me, but it's been a long time since I worked on Thunderbird, so I'm redirecting to, uh, let's say Magnus.

(My only suggestion would be that I think we want this to default to false, not true, since if people are taking the time to customize their colours, I suspect they mostly want other people to see them.)
Attachment #8910866 - Flags: review?(bwinton) → review?(mkmelin+mozilla)
(In reply to Blake Winton (:bwinton) (:☕️) from comment #6)
> Comment on attachment 8910866 [details] [diff] [review]
> Added new preference to select "Reader's default colors" for any new message
> 
> (My only suggestion would be that I think we want this to default to false,
> not true, since if people are taking the time to customize their colours, I
> suspect they mostly want other people to see them.)

My defense for that decision: I suspect that very few people have taken the time to customize their colors, but since nobody had the chance to select default colors, every user has "custom colors". If the user did have actual custom colors, he will notice the change as soon as he composes his first message, and will be able to set the option about his custom colors, and he won't have lost the color data he set, either.

If we decide to default to false, no problem, but I did have a line of reasoning for selecting true.
(In reply to palswim from comment #7)
> (In reply to Blake Winton (:bwinton) (:☕️) from comment #6)
> > (My only suggestion would be that I think we want this to default to false,
> > not true, since if people are taking the time to customize their colours, I
> > suspect they mostly want other people to see them.)
> 
> My defense for that decision: I suspect that very few people have taken the
> time to customize their colors, but since nobody had the chance to select
> default colors, every user has "custom colors". If the user did have actual
> custom colors, he will notice the change as soon as he composes his first
> message, and will be able to set the option about his custom colors, and he
> won't have lost the color data he set, either.
> 
> If we decide to default to false, no problem, but I did have a line of
> reasoning for selecting true.

I couldn't agree more.

On top of that, defaulting that pref to 'true' would mean that people would by default be sending emails with colours specified, regardless of whether or not they are taking time to customize their colours. In other words, the sender would have to take extra effort to explicitly honor recipient's font and colour settings. I really don't think this should ever be the case.
I think the selecting colored text is an anti-feature. Seriously, who would (and should) do that. Maybe we should consider removing it altogether.
Comment on attachment 8910866 [details] [diff] [review]
Added new preference to select "Reader's default colors" for any new message

Review of attachment 8910866 [details] [diff] [review]:
-----------------------------------------------------------------

Nits: please no trailing whitespace. (It's marked red if you look through the bugzilla review link). Also, 2-space indent please
Attached patch No Display Colors.patch (obsolete) — — Splinter Review 
Removed trailing whitespace and fixed four-line indentation in previous patch.
Attachment #8910866 - Attachment is obsolete: true
Attachment #8910866 - Flags: review?(mkmelin+mozilla)
Attachment #8915334 - Flags: review?(mkmelin+mozilla)
(In reply to Magnus Melin from comment #9)
> I think the selecting colored text is an anti-feature. Seriously, who would
> (and should) do that. Maybe we should consider removing it altogether.

We could change the option text to read "Use Reader's default colors (do not set colors). Seriously, do this, otherwise your friends will hate you."
(In reply to Magnus Melin from comment #9)
> I think the selecting colored text is an anti-feature. Seriously, who would
> (and should) do that. Maybe we should consider removing it altogether.

I do know of at least one legitimate use-case for backgrounds in messages: My company has an understanding that when a sender uses a background color, they intend for the message to stay internal to the company. So, if in a thread with external users, I want to reply to somebody internally and ensure that the recipient understands that the external parties shouldn't see the message, I use a background color.

However, this is a deliberate per-message decision, not a default.
Comment on attachment 8915334 [details] [diff] [review]
No Display Colors.patch

Review of attachment 8915334 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay. 
Would you be up for instead removing the setting color by default pref UI? I'd rather do that than add more UI that nobody should be using.
In fact, setting the default size in the pref also seems way weird, but that could be for another bug.
Attachment #8915334 - Flags: review?(mkmelin+mozilla)
Assignee: nobody → palswim
I removed the UI for the colors, though not the preferences themselves. Did you want me to remove the preferences as well?
Attachment #8915334 - Attachment is obsolete: true
Attachment #8919853 - Flags: review?(mkmelin+mozilla)
My apologies with the hasty add of the other attachment. This one contains valid patch data.
Attachment #8919853 - Attachment is obsolete: true
Attachment #8919853 - Flags: review?(mkmelin+mozilla)
Attachment #8920330 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8920013 [details] [diff] [review]
Removal of the options to specify defaults for background and text colors in a message

The MozReview check-in obsoletes this patch
Attachment #8920013 - Attachment is obsolete: true
Comment on attachment 8920330 [details]
Bug 690644 - Removal of the options to specify defaults for background and text colors in a
message

https://reviewboard.mozilla.org/r/191336/#review208612

So sorry for the review delay! Looks good in spirit, but some things to do still.

::: mail/locales/en-US/chrome/messenger/preferences/compose.dtd:24
(Diff revision 1)
>  <!ENTITY font.label                           "Font:">
>  <!ENTITY font.accesskey                       "n">
>  <!ENTITY size.label                           "Size:">
>  <!ENTITY size.accesskey                       "z">
> +<!ENTITY colorDefaults.label                  "Use Reader's default colors (do not set colors)">
> +<!ENTITY colorDefaults.accesskey              "D">

You're not using these.
Also, please remove the entities for the removed parts

::: mailnews/mailnews.js:798
(Diff revision 1)
>  pref("mailnews.customHeaders", "");
>  
>  // default msg compose font prefs
>  pref("msgcompose.font_face",                "");
>  pref("msgcompose.font_size",                "medium");
> +pref("msgcompose.default_colors",           true);

Actually, I think you should be changing the text_color and background_color prefs to "". 

Then when using them (https://dxr.mozilla.org/comm-central/rev/0fca2b6480744d708a6e4f541fc4607b174ea0e9/mail/components/compose/content/MsgComposeCommands.js#5904) you should not set the bgcolor and text attributes at all if empty. With your current patch we send <body text="" and bgcolor=""> which is not what we want.
Attachment #8920330 - Flags: review?(mkmelin+mozilla) → review-
[I do find a normal attached patch easier to review than using review-board]
message; r?mkmelin+mozilla@iki.fi
Changed the Thunderbird Composition preferences UI section to remove the options about
setting the default background and text colors in a message. Left the options intact and
added new "msgcompose.default_colors" options, which defaults to "true", which tells
Thunderbird not to use the default colors, but instead set no text or background color in a
new message. The user can change this in a message-specific way in the "Format > Page Colors
and Background..." dialog in the message composition window.

MozReview-Commit-ID: De4YF6Esezx
Attachment #8920330 - Attachment description: Bug 690644 - Removal of the options to specify defaults for background and text colors in a → Bug 690644 - Removal of the options to specify defaults for background and text colors in a message
Attachment #8920330 - Attachment is obsolete: true
(In reply to Magnus Melin from comment #19)
> Then when using them
> (https://dxr.mozilla.org/comm-central/rev/0fca2b6480744d708a6e4f541fc4607b174ea0e9/mail/components/compose/content/MsgComposeCommands.js#5904)
> you should not set the bgcolor and text attributes at all if empty. With your
> current patch we send <body text="" and bgcolor=""> which is not what we want.

Without the text and bgcolor values in the body control, the Editor doesn't seem to recognize its input as valid HTML. With the patch that left blank values in those properties, the message Editor would save the message as HTML (filename.html) that represented the input faithfully. With the patch that doesn't set those values, the Editor doesn't provide the option to save as HTML, and doesn't save any data in the file.

Thankfully, the message will send with the correct data; this problem only manifests itself on a save ("Save As > File...").

I think the state of having blank values in the attributes is better than the state of having a broken message save function. And, I think that the fix for the message editor that would recognize the BODY element without the color/bgcolor values as valid HTML would belong in another bug, which I can investigate after the completion of this one.
Attachment #8934741 - Attachment description: Removal of the options to specify defaults for background and text colors in a → Removal of the options to specify defaults for background and text colors in a message
Attachment #8934741 - Flags: review?(mkmelin+mozilla)
The "Save As" file appears to render the contents faithfully now, so we no longer have the issue I mentioned earlier.
Attachment #8934741 - Attachment is obsolete: true
Attachment #8934741 - Flags: review?(mkmelin+mozilla)
Attachment #8986848 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8986848 [details] [diff] [review]
Removal of the options to specify defaults for background and text colors in a message

Review of attachment 8986848 [details] [diff] [review]:
-----------------------------------------------------------------

Bitrotted a bit. I'll do some adjustments

::: mail/components/compose/content/MsgComposeCommands.js
@@ +5268,3 @@
>    try {
> +    textColor = (useDefault ? "" : getPref("msgcompose.text_color"));
> +    if ((!bodyElement.getAttribute("text")) && textColor.length)

instead of checking .length we check truthiness

::: mail/components/preferences/compose.xul
@@ +155,5 @@
> +              <spacer flex="1"/>
> +              <button id="restoreHTMLDefaultsButton"
> +                      label="&restoreHTMLDefaults.label;"
> +                      accesskey="&restoreHTMLDefaults.accesskey;"
> +                      oncommand="gComposePane.restoreHTMLDefaults();"/>

this button is now in a strange place
Attachment #8986848 - Flags: review?(mkmelin+mozilla)
Attached patch TB.patch (obsolete) — — Splinter Review 
Updated and ready for checkin.
Attachment #8986848 - Attachment is obsolete: true
Attachment #8996978 - Flags: review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
(In reply to Magnus Melin from comment #24)
> Comment on attachment 8986848 [details] [diff] [review]
> Removal of the options to specify defaults for background and text colors in
> a message
> 
> Review of attachment 8986848 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Bitrotted a bit. I'll do some adjustments

Thanks. Bitrot meaning that the Mozilla coding standards have changed since that code first appeared? I was trying to change as little as possible, but I do like your larger changes better.
Comment on attachment 8996978 [details] [diff] [review]
TB.patch

Review of attachment 8996978 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, why are you ripping out the UI to set those colours? You should have added your new preference just above and disabled the choice of colours when the "reader colours" are selected.

::: mail/components/preferences/compose.inc.xul
@@ +8,5 @@
>  
>      <stringbundle id="bundle_addressBook" src="chrome://messenger/locale/addressbook/addressBook.properties"/>
>  
>      <preferences id="composePreferences">
> +       <preference id="mail.preferences.compose.selectedTabIndex" name="mail.preferences.compose.selectedTabIndex" type="int"/>

Why this change? You're messing up the indentation :-(
Attachment #8996978 - Flags: feedback-
Attachment #8996978 - Flags: ui-review?(richard.marti)
Comment on attachment 8996978 [details] [diff] [review]
TB.patch

Apart from a one unnecessary hunk, I don't really like removing UI here. Richard, can you please take a look.

If anything, I'd implement:
[ ] Readers default colors - Text Color [ ] - Background Color [ ]
If the first one is checked, disable the other two. And also, don't change the behaviour for everyone, make your new pref 'false' by default.

The longer I look at this, the less I like it.
Attachment #8996978 - Flags: feedback- → review-
Comment on attachment 8996978 [details] [diff] [review]
TB.patch

Yes, I'm too against to remove this options because it makes for them that want to use this harder to set this colors. They need then to use a template and this isn't as easy as simply pressing on "Write". My wife for example uses this and this change would consternate her.

I like the "[ ] Readers default colors - Text Color [ ] - Background Color [ ]" with default to false more.
Attachment #8996978 - Flags: ui-review?(richard.marti) → ui-review-
(In reply to palswim from comment #26)
> Bitrot meaning that 

That the patch didn't apply due to code changes between the time you created the patch and now.
Jörg, Richard, what's not to like? :)

The whole point here is that we're sending out pretty useless junk in every single html mail we send. 

These colors are NOT being recognized by the general public as gmail and other big ones strip them out, for very good reason: nobody wants to be looking at strange colors all the sudden. 

For the clients where it's respected, recipients have to jump through hoops to avoid the color mishmash, when all they want to do is read through their mail.

Users who set the colors are not making them selves look good, nor Thunderbird.

And lastly, this is about using specific colors by default. It's fine to use colors for specific use cases in on-off cases to make a certain point, IMHO, but makes no sense as a default.
It's about HTML messages and when the colors aren't recognized by general public it's also not so severe. TB users can set simple HTML and all is good. If someone uses crazy colors, I'm sure the recipients would inform him about the bad choose.

I could be convinced to set the "Readers default colors" to true, but I'm against removing this functionality.
Our user base is *very* conservative, so there is no point for this needless change. TB has been like this since the beginning of time with few complaints. Besides, you'd create to undocumented and inaccessible preferences that way:
pref("msgcompose.text_color", "#000000");
pref("msgcompose.background_color", "#FFFFFF");
(In reply to Richard Marti (:Paenglab) from comment #32)
> It's about HTML messages and when the colors aren't recognized by general
> public it's also not so severe. TB users can set simple HTML and all is
> good. If someone uses crazy colors, I'm sure the recipients would inform him
> about the bad choose.

Well first of all, simple HTML is hardly to recommend for most people - they would just think Thunderbird can't render emails properly. Even besides that, why would you force people to take some action just so that all emails would look decent by default. Defaults are supposed to be helpful and just let users be happy without changing anything.
(In reply to Jorg K (GMT+2) from comment #33)
> Our user base is *very* conservative, 

Conservatives can't keep us from making the product better.

> change. TB has been like this since the beginning of time with few

Of course, this is all just remains from sharing code with the web page editor. "Because we can."
So how about this? Remove the UI for setting default colors and background so that 
 * user's can't shoot themselves in the foot (their mails look bad if the recipient's client actually shows it)
 * they don't have wrong expectations about what their email will look like to the recipient

But don't change the behavior for current users if they did set custom colors. It would still be possible to set custom colors through the hidden prefs if you're so inclined.
How will you determine whether the user has set custom colors because they wanted them that way or if they did so because there was no option not to set colors?

Could Thunderbird unset these hidden prefs once during upgrade, so that those who really want them, would have to set them again?
Forcing the user to set it in the hidden prefs is okay for experienced users but the average user doesn't know this exists. And when he goes there, the warning on about:config may let him fear to do something there. And then he has to know the correct color notation and can't choose from a color picker.

How about set "Readers default colors" to true as default and on false for current users if they did set custom colors? Users that never where in the prefs to set this settings don't see a difference because it's automatically disabled and the others can still use their colors.
(In reply to Richard Marti (:Paenglab) from comment #38)
> Forcing the user to set it in the hidden prefs is okay for experienced users
> but the average user doesn't know this exists.

Sure, but that's the idea here. To loose useless/harmful UI.

> How about set "Readers default colors" to true as default and on false for
> current users if they did set custom colors?

Yes that is basically what I'm proposing in comment 36. Only that we'd remove the UI to set colors. Existing user's would notice no difference.
(In reply to Magnus Melin from comment #39)
> > How about set "Readers default colors" to true as default and on false for
> > current users if they did set custom colors?
> 
> Yes that is basically what I'm proposing in comment 36. Only that we'd
> remove the UI to set colors. Existing user's would notice no difference.

I think we should leave the UI (and improve it with the new setting). Users never using this don't search for it and leave it unchanged, the ones using it still can find it, and maybe the new setting helps them to consider if they still want to use it or not. There exist surely also reasons to use it, and to make it harder to access to it through hidden prefs isn't the correct way to teach them they shouldn't use it.
> I think we should leave the UI (and improve it with the new setting).

+1 to this, and hopefully it can flip the default to "colour agnostic" in existing setups. Let's not go down the route of GNOME and remove every possible configuration dialog (that's already implemented!) just in case someone actually wants to use it.
I'd be in favor of setting the new "Use reader's default colors" pref to true for everyone. Those who care would just untick it, and those who don't care or no longer care would be sending cleaner emails without any extra work required. This could be mentioned in release notes.
Attached patch 690644-default-colors.patch (obsolete) — — Splinter Review 
Unless we want to defer this for another seven years, we could accept this compromise now.

Richard, all in the one line looks a bit squashy, perhaps you can beautify this a bit. Please upload a new patch in this case and obsolete mine.
Attachment #9001878 - Flags: ui-review?(richard.marti)
Attachment #9001878 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9001878 [details] [diff] [review]
690644-default-colors.patch

Let's not expand on an UI that should be removed.
(It would also have to be on a separate row.)

Testing it out now, allowing setting the default font size is also a pretty major UI mistake. Nobody should be setting that to anything but normal by default. If they have problems with eyesight they should use the zooming in the composition - not set the font size for the recipient. Yack - too many things inherited from Editor :(
Attachment #9001878 - Flags: review?(mkmelin+mozilla) → review-
Well, it goes on the WONTFIX pile then.
The original patch didn't as it never set the pref. I fixed this but now the clicking the button doesn't update the color button state. I tried to add a eventListener but it doesn't work. In this patch the listener is still there to fix it.

(In reply to Magnus Melin from comment #44)
> Testing it out now, allowing setting the default font size is also a pretty
> major UI mistake. Nobody should be setting that to anything but normal by
> default. If they have problems with eyesight they should use the zooming in
> the composition - not set the font size for the recipient. Yack - too many
> things inherited from Editor :(

Then maybe we should disable HTML mails. ;-)
The user can still set the size, colors etc. in the composer itself. He has then to do this on every new message, or use a template.
Attachment #9001878 - Attachment is obsolete: true
Attachment #9001878 - Flags: ui-review?(richard.marti)
(In reply to Richard Marti (:Paenglab) from comment #46)
> Then maybe we should disable HTML mails. ;-)

There are use cases for using various kinds of emphasis, picture and more. If you overuse it it's just bad.

> The user can still set the size, colors etc. in the composer itself. He has
> then to do this on every new message, or use a template.

Better template sure would be nice.
But, you're thinking about it from the wrong angle IHMO. It's not that we're creating a chore for the user, we're making it harder to do something he shouldn't do.
(In reply to Magnus Melin from comment #44)
> Testing it out now, allowing setting the default font size is also a pretty
> major UI mistake. Nobody should be setting that to anything but normal by
> default.
Like Richard, I disagree. We should remove HTML e-mail altogether and we'd get rid of a lot of problems.

(In reply to Richard Marti (:Paenglab) from comment #46)
> The original patch didn't as it never set the pref.
I don't understand. My patch was working, no? What was wrong? Only the one like looked squashy?
WONTFIX due to lack of consensus.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Anecdotal evidence from #maildev just a few days ago: https://mozilla.logbot.info/maildev/20180815#c15170820
(In reply to Jorg K (GMT+2) from comment #49)
> WONTFIX due to lack of consensus.

Can we just agree to implement the patch that adds the flag to use default colors to the Preferences and then argue about whether or not to remove that entire UI from the Preferences in a separate Bug?

Having the option (defaulting it to true?) is much better for everyone than the current state.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(In reply to palswim from comment #51)
> Can we just agree to implement the patch that adds the flag to use default
> colors ...
Apparently not. That's what you and I and Richard suggested and I prepared a patch for (maybe not fully working), but Magnus is blocking it. He wants the original patch but some are opposed to removing the existing functionality. So typically in deadlock situations like this, nothing happens since the project is build on consensus. For an even sadder case, see bug 683809 where some urgently needed functionality can't get implemented since people can't agree on the UI for it. It's grotesque since the original bad state is maintained forever and people go out and start writing add-ons to work around it. You could write an add-on that went and removed the body colours for every message ;-)

About a year ago it was decided to establish an "Engineering Steering Committee" so solve cases like this via a vote if consensus cannot be reached, read: https://mail.mozilla.org/pipermail/tb-planning/2018-August/006131.html

So maybe in a few months from now we can move this forward.
I don't think there's a lack of consensus on the change that comment 51 proposes.

Also, I fail to see how Markus is blocking anything. He's voicing his discontent with the fact that the options considered harmful are staying, but that's not same as blocking the patch AFAICS.
One of us cannot read :-( - I read:

> Let's not expand on an UI that should be removed.
r-

Since we need a positive review to land it, I'd call this "blocking".
Ah, indeed, I didn't notice the r-. But he also said
> (It would also have to be on a separate row.)
Maybe (hopefully) that's the reason?
Attachment #9001951 - Attachment description: 690644-default-colors.patch → 690644-default-colors.patch (v2, RM)
No, I didn't want to expand on an UI that should be removed. 

How does adding the pref help? That would be a textbook case of design by committee - adding a checkbox that a few people can select but for the large masses, no improvement. The average user would have no idea what to select. To make an informed decision you have to understand how color selection in the front end happens. 

I already presented good reasons for why to remove, but so far lacking response to the details - except for that it would make it harder for the user to use this feature. But the whole point was exactly to make it harder, so that you don't shoot yourself in the foot, or have false hopes it will show up. I'd like to hear why we should allow that. 

What are the (good) use cases for sending *by default* mails with font=huge, or background=red for that matter?
OK. Here's the last attempt.

- Fixed: preference value not set when box was checked/unchecked.
- Labels are also disabled now.
- Restore defaults button moved down since it resets all the items above.

So if this is not not good enough, we really need to come to an ESC decision on this.

I see Magnus wrote another comment, so I'll answer that later.
Attachment #9001951 - Attachment is obsolete: true
Attachment #9002095 - Flags: ui-review?(richard.marti)
Attachment #9002095 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9002095 [details] [diff] [review]
690644-default-colors.patch (v3, JK)

ui-r+ with moving the "Restore Defaults" button to the right by adding a <spacer flex="1"/>.
Attachment #9002095 - Flags: ui-review?(richard.marti) → ui-review+
(In reply to Magnus Melin from comment #56)
OK, here another attempt of an answer.

This feature has been in the box since forever and we have no data on how may people are using it. You will most likely cause an outcry when you remove it, just like we did with the correspondents columns where we had to back-paddle 100%, the "paragraph by default" (which you can fortunately switch off, but how many bugs did that generate) and the mailing list reply. Sadly you don't deal with the unhappy users, but I do. 

> No, I didn't want to expand on an UI that should be removed.
So far, you're the only person saying that it "should" be removed. 

> I already presented good reasons for why to remove, but so far lacking
> response to the details - except for that it would make it harder for the
> user to use this feature. But the whole point was exactly to make it harder,
> so that you don't shoot yourself in the foot, or have false hopes it will
> show up. I'd like to hear why we should allow that.
I don't get the point. Until now, I've used TB for eight years with the default options and I don't think I shot myself in the foot so far. TB is a HTML-capable editor, and setting text, background and link colours in the body element is *valid* HTML. If it gets ignored by some clients, that's not our problem, we don't want to dumb down to the lowest common denominator. That we render all HTML in quirks mode however *is* our problem, since some HTML e-mail is not displayed as intended.

> What are the (good) use cases for sending *by default* mails with font=huge,
> or background=red for that matter?
None. But there are cases for people who wish to send e-mail with a - say - pastel background, some non-black text in a commonly supported font in a slightly modified size. Web mailers allow that configuration.

Once again:
- The current defaults work and with this patch they will work even better.
- We must not remove this feature unless we have proof that it's not used by 99.9% of users.
- The additional option makes the situation better for the "standard user" who doesn't care.
Now with spacer as requested.
Attachment #9002095 - Attachment is obsolete: true
Attachment #9002095 - Flags: review?(mkmelin+mozilla)
Attachment #9002103 - Flags: ui-review+
Attachment #9002103 - Flags: review?(mkmelin+mozilla)
Hotmail shows the background colour, not the text colour, Gmail shows none. It's quite irrelevant really. If you want to protect against e-mail not being rendered as intended at the recipient's end, the only thing you can do is disable HTML altogether. And best only allow plain ASCII since anything non-ASCII is known to fail every once in a while, so we shouldn't pretend we can actually send that stuff when the recipient doesn't get it, or worse, gets mojibake.
GMX shows both colours.
> So far, you're the only person saying that it "should" be removed. 
Actually, I think it should be removed as well. I just don't think that the discussion on whether or not it should be removed should actually block any interim improvements (and the ability to not specify colors is certainly an improvement).

Also,
> You will most likely cause an outcry when you remove it
is most likely a huge exaggeration.

Also, if we leave the prefs working, just hidden, whoever truly wants to shoot themselves in the foot and annoy people with their color choices will still be able to keep doing so, it's just that this possibility will be demoted from "we endorse this" to "we do not endorse this".

And the comments about HTML being valid or not… I don't think they're relevant in this case. There's a whole lot of valid HTML which no decent e-mail client will (or should) allow, and for a good reason. 

But again, if no consensus can be reached on removing this UI, at least a possibility to use the defaults should be added as an interim solution IMO.
(In reply to Rimas Kudelis from comment #63)
> is most likely a huge exaggeration.
Let's not try to second-guess statistical data we don't have. As you said "most likely". We can't ship software to 25 million users on the principle of "most likely". At the beginning of comment #59 I listed three changes that did cause and outcry, but in fairness, I must admit that this feature is "most likely" less used.

The prime problem is: Unlike FF we have zero telemetry data and we don't know that our users are doing. We know however from general experience, that the user base is conservative, as in "likes to conserve existing features", and quite resistant to change.
(In reply to Magnus Melin from comment #50)
> Anecdotal evidence from #maildev just a few days ago:
> https://mozilla.logbot.info/maildev/20180815#c15170820

"Look at the size of my text from your response email -- it's huge! I do not understand how Thunderbird manages size of text at all. I'm typing this and it looks really tiny. Yet if I save the message as a Draft and then view the message in the Drafts folder it looks okay. But if I chose to Edit the draft it's back to looking tiny again."

We don't really know what happened there, do we? Looking tiny while you type and OK when viewed seems more like a zoom problem, no?

Anyway, I've discussed this with Magnus. IMHO, letting users select fonts is the biggest foot-gun we have in the system, since the recipient will most likely *not* have those fonts installed since many of them are not at all cross-platform.

I think the HTML section in the preferences should come with a big disclaimer and I will suggest one now.
Added a warning:
  Warning: Using non-default values may cause incorrect message display when viewed by the recipient
If you have a better/shorter/more precise suggestion, let me know. We can now discuss seven months about the wording ;-(
Attachment #9002390 - Flags: ui-review?(richard.marti)
Attachment #9002390 - Flags: review?(mkmelin+mozilla)
Attachment #9002390 - Attachment description: 690644-default-colors.patch → 690644-default-colors.patch (v3c, JK, with warning)
Comment on attachment 9002390 [details] [diff] [review]
690644-default-colors.patch (v3c, JK, with warning)

Review of attachment 9002390 [details] [diff] [review]:
-----------------------------------------------------------------

Let's see what Magnus says.

::: mail/components/preferences/compose.inc.xul
@@ +107,5 @@
>  
>            <groupbox>
>              <caption label="&htmlComposeHeader.label;"/>
>              <hbox align="center">
> +              <label value="&htmlWarning.label;"/>

Don't put it in a <hbox> and use
 <description>&htmlWarning.label;</description>
instead. Then the text would wrap when window is too narrow.

::: mail/locales/en-US/chrome/messenger/preferences/compose.dtd
@@ +15,5 @@
>  <!ENTITY addExtension.label                   "add extension to file name">
>  <!ENTITY addExtension.accesskey               "e">
>  
>  <!ENTITY htmlComposeHeader.label              "HTML">
> +<!ENTITY htmlWarning.label                    "Warning: Using non-default values may cause incorrect message display when viewed by the recipient">

Shouldn't there be a full stop?
Attachment #9002390 - Flags: ui-review?(richard.marti) → ui-review+
I wouldn't put one since it's more like a headline/banner.
Shorter warning proposal:

Warning: Non-default values may cause incorrect message display at the recipient!

... at | for | with the recipient
Using <description>&htmlWarning.label;</description> now; shortened warning by removing "Using". Thomas, thanks for the suggestion, but the preposition at|for|with is a real dilemma. I think "when viewed by" is still best. It fits well onto the screen.
Attachment #9002103 - Attachment is obsolete: true
Attachment #9002390 - Attachment is obsolete: true
Attachment #9002103 - Flags: review?(mkmelin+mozilla)
Attachment #9002390 - Flags: review?(mkmelin+mozilla)
Attachment #9003085 - Flags: ui-review+
Attachment #9003085 - Flags: review?(mkmelin+mozilla)
(In reply to Jorg K (GMT+2) from comment #71)
> Created attachment 9003086 [details]
> 690644.png - screenshot of (v3d)

I posted the following comment on the maildev list[1]. Posting here (with some edits) for completeness.


I like the screenshot in the context of the main program preferences area. It makes sense.

I just have two caveats (and I apologise if these issues have already been covered here):
(1) Use of the phrase "non-default" always causes me to ask: "Hang on, what are the defaults? It doesn't say. But I don't want to lose my current settings by clicking Restore Defaults just to find out." And so rather than using "non-default", might I suggest that the warning text is something like this: "Warning: Removing the tick from 'Use reader's default colors' may cause incorrect message display when viewed by the recipient".

(2) The use of "recipient" and "reader" to refer to essentially the same entity is I think inconsistent. Furthermore, the word "reader" is often used to refer to particular software or parts of software and so I don't think it is necessarily clear that "reader" refers to the recipient's mail client in this context. For this reason I suggest changing "Use reader's default colors" to "Use the recipient's default colors" or even "Do not set colors (allow recipient to view using their own color scheme)".

And two comments about context:
(1) If this is the look for the main program preferences area then it does of course need to be matched by the UI in the compose window. I.e. When 'Use reader's default colours' option is selected, the per-message colour picker in the compose window needs to be disabled (not removed!) and the user needs to be told why this is (as per items (i) and (j) in my previous message[2]).

(2) Ideally in my opinion there would be a per-message option somewhere in the compose window to enable/disable setting text and background colours (using similar language to whatever is decided for the program preferences area under discussion here) but as I mentioned in item (h) before[2] I think it can be on a menu or in a dialog by default rather than being immediately exposed on a toolbar.


Footnote:-
1: http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2018-August/001285.html

2: http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2018-August/001283.html
I had to rebase this since linting and the colour picker changes shredded right through it. I also removed "Warning" from the UI, now it's just a note.

When testing this, note bug 1495808 comment #13, reset of the background colour not working a second time.

Magnus, for how much longer do you intend to block this solution which represents an improvement for all TB users, although it might not be the perfect solution; two years or indefinitely like in bug 683809? This is seriously frustrating, not to mention the extra work this causes by having to rebase and retest patches over and over. Let me know if I should find another reviewer who will gladly accept this. Needless to mention that the solution is backed by other peers and sub-module owners.
Attachment #9003085 - Attachment is obsolete: true
Attachment #9003085 - Flags: review?(mkmelin+mozilla)
Attachment #9015002 - Flags: ui-review?(richard.marti)
Attachment #9015002 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9015002 [details] [diff] [review]
690644-default-colors.patch (v3e, JK) with note - rebased

Thanks!
I think too, we should go this way.
Attachment #9015002 - Flags: ui-review?(richard.marti) → ui-review+
+1 for not letting perfect be the enemy of better


Outlook has "stationery" options (1) and if you remove these features a subset of people will be unhappy.

Unlike Thunderbird though, Outlook doesn't make the mistake of setting explicit styles by default in HTML email.


I would quite like a "Use Custom Styles" checkbox in the composition prefs which entirely hid the font / text colour controls when disabled (by default) - would that notion please the people who want to see the dialog removed completely? 

I would find this clearer than "Use reader's default colours" which I had to think carefully about the implications of. If the custom font/colour elements were hidden or greyed as in the screenshot above, this underscores the notion that they are not being used.


1. https://support.office.com/en-us/article/apply-stationery-backgrounds-or-themes-to-email-messages-e24fc237-62e1-4361-82a3-d8a7467d2b7e
Carrying forward Richard's ui-r+.

Looks like Magnus has been too busy to review this for over two months now, so let's get another reviewer.
Attachment #8996978 - Attachment is obsolete: true
Attachment #9015002 - Attachment is obsolete: true
Attachment #9015002 - Flags: review?(mkmelin+mozilla)
Attachment #9020698 - Flags: ui-review+
Attachment #9020698 - Flags: review?(acelists)
Comment on attachment 9020698 [details] [diff] [review]
690644-default-colors.patch (v3e, JK) with note - rebased a second time

Review of attachment 9020698 [details] [diff] [review]:
-----------------------------------------------------------------

I suppose this is a step in the right direction.

::: mail/locales/en-US/chrome/messenger/preferences/compose.dtd
@@ +15,5 @@
>  <!ENTITY addExtension.label                   "add extension to file name">
>  <!ENTITY addExtension.accesskey               "e">
>  
>  <!ENTITY htmlComposeHeader.label              "HTML">
> +<!ENTITY htmlWarning.label                    "Non-default values may cause incorrect message display when viewed by the recipient">

let's not add this

@@ +21,5 @@
>  <!ENTITY font.accesskey                       "n">
>  <!ENTITY size.label                           "Size:">
>  <!ENTITY size.accesskey                       "z">
> +<!ENTITY useReaderDefaults.label              "Use reader's default colors">
> +<!ENTITY useReaderDefaults.accesskey          "r">

r is already taken
Attachment #9020698 - Flags: review?(acelists) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/29f023e0da21
Don't specify background and text colors by default when composing a message. r=mkmelin DONTBUILD
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Landed with the warning removed and the access key changed to "d" for "default". Sadly it underlines the "d" in "reader's".
Target Milestone: --- → Thunderbird 65.0
Blocks: 1641225
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: