Closed Bug 1638720 Opened 9 months ago Closed 8 months ago

Chat entry area doesn't respect background colour

Categories

(Thunderbird :: Instant Messaging, defect)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 78.0

People

(Reporter: ari, Assigned: Paenglab)

Details

Attachments

(5 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Change the background colour the black and the text colour to white within the preferences.

Actual results:

The chat history shows in the chosen colours. The chat message entry area shows with the chosen text colour (white) but retains the white background also, instead of using black.

Obviously this makes the dark theme impossible to use.

Expected results:

Black background.

Alex or Richard -- do you know what might be the problem here?

Flags: needinfo?(richard.marti)
Flags: needinfo?(alessandro)

Change the background colour the black and the text colour to white within the preferences.

What does this mean?
Did you change the Chat style to Dark, or did you change the entire Thunderbird Theme to dark?

Flags: needinfo?(alessandro) → needinfo?(ari)

I think it's about the conversation textbox on bottom not changing to dark and the vertical tabs staying white.

Flags: needinfo?(richard.marti)
Attached image Dark theme.
Flags: needinfo?(ari)
Attached image dark theme colours
Attached image chat theming

The problem isn't changed by going from normal default theme to dark theme or back. Also, none of the settings in the chat styling make any difference.

The chat entry area is always white, with white text. This is OSX 10.15.4 with the operating system in dark mode.

Attached patch 1638720-themeable-chat.patch (obsolete) — Splinter Review

This makes the remaining chat areas themeable:

  • The conversation box (there you enter your text).
  • The "No account connected" screen.
  • The vertical tabs.

I added also a dark thunderbird chat theme variant. You have to select it manually in the prefs. Aleca, can you check, if the colours are okay for the dark theme? For Twitter I'm using the same colours.

Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9151465 - Flags: review?(alessandro)
Comment on attachment 9151465 [details] [diff] [review]
1638720-themeable-chat.patch

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

This looks very good, thank you so much.
A little nit update:
Let's implement a consistent `margin: 4px` to the `#goToConversation` button.

Since you're adding extra variations, it would be better to ping Patrick or Florian for a full review as they have a better understanding of the ins and outs of the chat area.
Attachment #9151465 - Flags: review?(alessandro) → ui-review+

Added the 4px around the #goToConversation.

Clokep, I added a dark variant to the Thunderbird Chat theme. Please can you check that I have done it correctly.

When I'm at it, can you say me for what the Show Headercheckmark is in the prefs? The messenger.options.messagesStyle.showHeader pref seems to be not used in TB.

Attachment #9151465 - Attachment is obsolete: true
Attachment #9151562 - Flags: ui-review+
Attachment #9151562 - Flags: review?(clokep)

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/4b3e05877240
Implement the hover state for all tree rows on Linux and Mac. r=aleca

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

Sh.., I had the wrong bug in the commit message.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Comment on attachment 9151562 [details] [diff] [review]
1638720-themeable-chat.patch

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

It looks like the variant was added properly. I have one question, but r+.

At least for Instantbird the "Show Header" option displayed a bit more information in some themes, I don't think we ever shipped a theme that supported this though (only the converted Adium themes supported it).

The useful information that was generally shown in that was the info about who you were chatting with, but we have that built into the chrome (in the sidebar). https://developer.mozilla.org/en-US/docs/Mozilla/Chat_Core/Message_Styles#Theme_files talks about this a little bit it seems.

::: chat/modules/imThemes.jsm
@@ +502,5 @@
>        }
>      }
>  
>      // Fallback to the theme's default icons.
> +    return (aMsg.incoming ? "Incoming" : "Outgoing") + "/buddy_icon.svg";

Weird, I had no idea that only this theme used an icon... seems fine though.

::: mail/installer/allowed-dupes.mn
@@ +24,5 @@
>  chrome/messenger/skin/classic/messenger/messages/simple/Variants/Normal.css
>  chrome/messenger/skin/classic/messenger/messages/simple/Incoming/Context.html
>  chrome/messenger/skin/classic/messenger/messages/simple/Incoming/NextContext.html
> +chrome/messenger/skin/classic/messenger/messages/mail/Incoming/buddy_icon.svg
> +chrome/messenger/skin/classic/messenger/messages/mail/Outgoing/buddy_icon.svg

I only see this for Incoming, is it also meant to exist for Outgoing?
Attachment #9151562 - Flags: review?(clokep) → review+

(In reply to Patrick Cloke [:clokep] from comment #14)

Comment on attachment 9151562 [details] [diff] [review]
1638720-themeable-chat.patch

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

It looks like the variant was added properly. I have one question, but r+.

At least for Instantbird the "Show Header" option displayed a bit more
information in some themes, I don't think we ever shipped a theme that
supported this though (only the converted Adium themes supported it).

The useful information that was generally shown in that was the info about
who you were chatting with, but we have that built into the chrome (in the
sidebar).
https://developer.mozilla.org/en-US/docs/Mozilla/Chat_Core/
Message_Styles#Theme_files talks about this a little bit it seems.

Thanks. Then I'll check if we could remove it.

::: mail/installer/allowed-dupes.mn
@@ +24,5 @@

chrome/messenger/skin/classic/messenger/messages/simple/Variants/Normal.css
chrome/messenger/skin/classic/messenger/messages/simple/Incoming/Context.html
chrome/messenger/skin/classic/messenger/messages/simple/Incoming/NextContext.html
+chrome/messenger/skin/classic/messenger/messages/mail/Incoming/buddy_icon.svg
+chrome/messenger/skin/classic/messenger/messages/mail/Outgoing/buddy_icon.svg

I only see this for Incoming, is it also meant to exist for Outgoing?

At https://searchfox.org/comm-central/rev/eab31101bb58c46fdef567e3dcbcc0ef1fdbf3d8/mail/components/im/jar.mn#29-30 the same icon is packaged for Incoming and Outgoing

Target Milestone: --- → Thunderbird 78.0

(In reply to Richard Marti (:Paenglab) from comment #15)

At least for Instantbird the "Show Header" option displayed a bit more
information in some themes, I don't think we ever shipped a theme that
supported this though (only the converted Adium themes supported it).

Thanks. Then I'll check if we could remove it.

We can likely remove the UI to flip the preference, yes. Please CC me on the bug if you file one!

At https://searchfox.org/comm-central/rev/eab31101bb58c46fdef567e3dcbcc0ef1fdbf3d8/mail/components/im/jar.mn#29-30 the same icon is packaged for Incoming and Outgoing

Doh! Packaging magic. Makes sense! Thanks.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0c5827184c46
Make all chat areas themeable and add a dark Thunderbird chat theme variant. r=clokep ui-r=aleca

Status: ASSIGNED → RESOLVED
Closed: 8 months ago8 months ago
Resolution: --- → FIXED
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/b690a4a2500e
follow-up - Fix CSS parsing error. rs=me DONTBUILD
You need to log in before you can comment on or make changes to this bug.