Closed Bug 1641519 Opened 8 months ago Closed 2 months ago

mail.citation_color etc. are not respected during composition

Categories

(MailNews Core :: Composition, defect)

defect

Tracking

(thunderbird_esr78 wontfix, thunderbird84 affected)

RESOLVED FIXED
85 Branch
Tracking Status
thunderbird_esr78 --- wontfix
thunderbird84 --- affected

People

(Reporter: sexxxenator, Assigned: Paenglab)

Details

(Whiteboard: [dupme])

Attachments

(2 files, 4 obsolete files)

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

Component: Untriaged → Composition
Product: Thunderbird → MailNews Core
Whiteboard: [dupme]

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?

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

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; }

Flags: needinfo?(mkmelin+mozilla)
Summary: Color are not respected → mail.citation_color etc. are not respected during composition

Something like this?

Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9189185 - Flags: review?(mkmelin+mozilla)

Fixed the linting.

Attachment #9189185 - Attachment is obsolete: true
Attachment #9189185 - Flags: review?(mkmelin+mozilla)
Attachment #9189339 - Flags: review?(mkmelin+mozilla)

Updated after landing of bug 1675012.

Attachment #9189339 - Attachment is obsolete: true
Attachment #9189339 - Flags: review?(mkmelin+mozilla)
Attachment #9189343 - Flags: review?(mkmelin+mozilla)
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}
}
Attachment #9189343 - Flags: review?(mkmelin+mozilla)

(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.

Attachment #9189343 - Attachment is obsolete: true
Attachment #9189731 - Flags: review?(mkmelin+mozilla)
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
Attachment #9189731 - Flags: review?(mkmelin+mozilla)

(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. :-)

Nevermind, i figured it out.

Attachment #9189731 - Flags: review+
Target Milestone: --- → 85 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/3c8996f7d3ae
Set the style for quoted text defined in preferences to plain text composertoo. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

(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.

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

Flags: needinfo?(mkmelin+mozilla)

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.

Attachment #9189731 - Attachment is obsolete: true
Flags: needinfo?(mkmelin+mozilla)
Attachment #9190642 - Flags: review?(richard.marti)

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

Thanks. You surely run the tests.

Attachment #9190642 - Flags: review?(richard.marti) → review+

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

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

Status: REOPENED → RESOLVED
Closed: 2 months ago2 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.