1641519 - mail.citation_color etc. are not respected during composition

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[L.I.A.R.]

Attachment: Whatever color conifg, color of quoted text is always bright blue

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

Hi,

I'm under XUbuntu 20.04, with a dark theme (awaita-dark).

ThB 68.7.0 (64-bit)

With email as plain text selected almost everywhere.

Whatever Preferences > Display > Colors selections I make ("When Displaying Quoted Plain Text message", or in "Use System Color"+Always, or anything else)....

Actual results:

...the quoted text in my email responses or forwards is in a bright blue color (similar as the one used for links).
On a very dark background this is totally unreadable.

Expected results:

Colors should either adapt to WM theme or respect the one in ThB's preferences

Comment 1

[L.I.A.R.]

I can confirm the bug is still present in 68.10.0 (64-bit).

Any news when we (users) will be able to get the correct (configured) colours used in the email composition window?

Comment 2

[L.I.A.R.]

I can confirm bug still present...

Since this bug is very blocking (either you have to switch back to light theme at the risk of burning your eyes, either you can't respond to any email) it would be great if its priority would be raised....

FWIW and for those that would encounter the same problem, I've found a (very annoying) work around:

• Select the whole text : Ctrl-a
• Cut the whole text : Ctrl-x
• Paste the whole text : Ctrl-v

It's probably due to a second bug (pasted content is not parsed and '>' are not interpreted as quoted text, thus not colored in blue), but pasted text is then in light-grey/white instead of blue which is now readable in dark mode

Comment 3

[Richard Marti (:Paenglab)]

This is no regression, tested on TB 60 and it happens too.

The settings for quoted plain text messages apply only on displayed messages and not in composer.

Magnus, could it be somehow possible to apply the prefs mail.quoted_style, mail.quoted_size and mail.citation_color also to the compose editor? Or at least only mail.citation_color.

A workaround would be to add this line to userContent.css (not userChrome.css):
span[_moz_quote="true"] { color: red; }

Comment 4

[Magnus Melin [:mkmelin]]

Likely possible, see https://stackoverflow.com/questions/13528512/modify-a-css-rule-object-with-javascript
Maybe around here: https://searchfox.org/comm-central/rev/862e050dfc9ae7a215e7b413b2b887df6dc77e17/mail/components/compose/content/MsgComposeCommands.js#8601

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1641519-quote-styles-to-composer.patch

Something like this?

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1641519-quote-styles-to-composer.patch

Fixed the linting.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: 1641519-quote-styles-to-composer.patch

Updated after landing of bug 1675012.

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 9189343 [details] [diff] [review]
1641519-quote-styles-to-composer.patch

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

Didn't try it yet, but yes, something like that

::: mail/components/compose/content/MsgComposeCommands.js
@@ +8655,5 @@
> +  style.sheet.insertRule(
> +    `span[_moz_quote="true"] { color: ` +
> +      Services.prefs.getCharPref("mail.citation_color") +
> +      `; ` +
> +      fontStyle,

should be ; no?
Since backtick allows multiline strings, it could be clearer to also define a variable for color, then 

span[_moz_quote="true"] {
  ${fontStyle}
  ${fontSize}
  ${citationColor}
}

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1641519-quote-styles-to-composer.patch

(In reply to Magnus Melin [:mkmelin] from comment #8)

Comment on attachment 9189343 [details] [diff] [review]
1641519-quote-styles-to-composer.patch

Review of attachment 9189343 [details] [diff] [review]:

Didn't try it yet, but yes, something like that

::: mail/components/compose/content/MsgComposeCommands.js
@@ +8655,5 @@

• style.sheet.insertRule(
• span[_moz_quote="true"] { color: +

•  Services.prefs.getCharPref("mail.citation_color") +
  

•  `; ` +
  

•  fontStyle,
  

should be ; no?

No, it is in the variable itself.

Since backtick allows multiline strings, it could be clearer to also define
a variable for color, then

span[_moz_quote="true"] {
${fontStyle}
${fontSize}
${citationColor}
}

Done like this.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9189731 [details] [diff] [review]
1641519-quote-styles-to-composer.patch

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

The option "only when plain text messages" mentioned seem not to exist anymore. 
I suppose these options should only apply when relevant conditions are set in that dropdown? And what about non-plain text?

Seems to work fine for plain text though

Comment 11

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin [:mkmelin] from comment #10)

The option "only when plain text messages" mentioned seem not to exist
anymore.

Not sure what you mean here.

I suppose these options should only apply when relevant conditions are set
in that dropdown? And what about non-plain text?

It will behave exactly like when reading messages. Only plain text messages use this options. This means also reading and writing messages look the same, what wasn't until now.

HTML messages use their own styles and aren't affected by this patch.

Seems to work fine for plain text though

Yes. :-)

Comment 12

[Magnus Melin [:mkmelin]]

Nevermind, i figured it out.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3c8996f7d3ae
Set the style for quoted text defined in preferences to plain text composertoo. r=mkmelin

Comment 14

[Geoff Lankow (:darktrojan)]

Adding this stylesheet to plain-text messages causes them to send as HTML messages. That's not a problem for browser_ext_compose_onBeforeSend.js (it doesn't care which format is used but now the result's in a different location) but it is for browser_sendFormat.js.

I'm going to back this out so it's easier to see if anything else breaks while you're working on a solution.

Comment 15

[Geoff Lankow (:darktrojan)]

Backed out:
https://hg.mozilla.org/comm-central/rev/f56bb2d0ec9843355ee36403352406533c8ce886

Comment 16

[Richard Marti (:Paenglab)]

(In reply to Geoff Lankow (:darktrojan) from comment #14)

Adding this stylesheet to plain-text messages causes them to send as HTML messages. That's not a problem for browser_ext_compose_onBeforeSend.js (it doesn't care which format is used but now the result's in a different location) but it is for browser_sendFormat.js.

I tried it again and it doesn't send as HTML. What does make that the test thinks it will send as HTML? The style should be only injected to the editor not the file.

Comment 17

[Richard Marti (:Paenglab)]

Magnus, please can you help here on the test? When I send a message it isn't sent as HTML.

Comment 18

[Magnus Melin [:mkmelin]]

Attachment: 1641519-quote-styles-to-composer.patch

The problem was that you added the stylesheet also for html compose, and for that, it changed the convertability. I put it inside !gMsgCompose.composeHTML.

Comment 19

[Richard Marti (:Paenglab)]

Comment on attachment 9190642 [details] [diff] [review]
1641519-quote-styles-to-composer.patch

Thanks. You surely run the tests.

Comment 20

[Magnus Melin [:mkmelin]]

mail/components/extensions/test/browser/browser_ext_compose_onBeforeSend.js and mail/test/browser/composition/browser_sendFormat.js pass locally.

Set off a try run now: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=25e1250c7e10b6521751ebbc285816500b04c0f1

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/40177245857a
Set the style for quoted text defined in preferences to plain text composertoo. r=mkmelin