Closed Bug 1830609 Opened 2 years ago Closed 1 year ago

Incorrect colours for WebExtension themes

Categories

(Thunderbird :: Theme, defect)

Thunderbird 114
defect

Tracking

(thunderbird115 fixed)

RESOLVED FIXED
116 Branch
Tracking Status
thunderbird115 --- fixed

People

(Reporter: max.m, Assigned: Paenglab)

References

Details

(Keywords: regression, Whiteboard: [Supernova3p])

Attachments

(4 files)

Attached file outlook_2013_mod.xpi

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/112.0.0.0 Safari/537.36

Steps to reproduce:

The documentation at https://webextension-api.thunderbird.net/en/stable/theme.html states the following:

frame: The background color of the header area.

tab_selected: Background color of the selected tab. Defaults to the color specified by toolbar.
tab_text: The text color for the selected tab. Defaults to the color specified by toolbar_text.
tab_background_text: The text color of the unselected tabs.

sidebar: The background color of the trees.
sidebar_text: The text color of the trees. Needed to enable the tree theming.

toolbar: The background color of the toolbars. Also used as default value for tab_selected.
toolbar_text: The text color in the main Thunderbird toolbar. Also used as default value for icons and tab_text.

Let's go with this example (see attachment):
"frame": "blue",
"tab_selected": "magenta",
"tab_text": "white",
"tab_background_text": "black",
"sidebar": "pink",
"sidebar_text": "green",
"toolbar": "yellow",
"toolbar_text": "red"

Observed behaviour in TB 114:
"frame": "blue", - menu (good), unselected tabs (good), unified toolbar (bad)
"tab_selected": "magenta", - selected tab (good)
"tab_text": "white", - selected tab (good)
"tab_background_text": "black", - unselected tabs (good), unified toolbar text (bad)
"sidebar": "pink", - folder tree (good), thread tree missing the colours (bad)
"sidebar_text": "green", - folder tree text (good)
"toolbar": not used (bad), used correctly as default when "tab_selected" is not specified (good).
"toolbar_text": not used (bad), used correctly as default when "tab_text" is not specified (good).

The following faults are observed:
Message header and quick filter bar and their text can't be styled at all any more.
The new unified toolbar has the wrong colours.
The tread tree can't be styled any more, the "sidebar" colour only applies the column header.

For the record, observed behaviour TB 102:
"frame": "blue", - menu (good), unselected tabs (good)
"tab_selected": "magenta", - selected tab (good) and mail toolbar (bad)
"tab_text": "white", - selected tab (good)
"tab_background_text": "black", - unselected tabs (good), quick filter bar text (bad) and message header text (bad)
"sidebar": "pink", - trees (good)
"sidebar_text": "green", - tree text (good)
"toolbar": "yellow", quick filter bar (good), message header (bad)
"toolbar_text": "red" - mail toolbar text

This is quite inconsistent:
The message header follows "toolbar" (yellow) and "tab_background_text" (black).
The mail toolbar follows "tab_selected" (magenta) and "toolbar_text" (red).
The QFB follows "toolbar" (yellow) and "tab_background_text" (black).

Marking this as regression although the TB 102 behaviour also wasn't correct.

Blocks: tb-new-3pane
Keywords: regression
Whiteboard: [Supernova3p]
Attached image we colours.png

Correction/addition: Other windows should be checked as well. For example, "toolbar" and "toolbar_text" apply to parts of the compose window. Most of the deficiencies described in comment #0 can also be seen in this picture.

Thanks for the report.

We need to be strict on the keywords here. If this has not been introduced by supernova, then we must not set the whiteboard flag and also not the blocking bug.

If this is wrong in 102 already, then we also have to set the reported version to 102, not 114, otherwise the regression window implicitly defined by the regression keyword is wrong.

Could you update all flags to exactly match your findings?

I think there is no point worrying about 102 here. For SN the UI was completely changed, so a 114/115 fix will likely not be backported. I suggest to focus on the regression part:

Message header and quick filter bar and their text can't be styled at all any more (was stylable in 102).
The new unified toolbar has the wrong colours (had different colours in 102, certainly not the "frame" colour).
The tread tree can't be styled any more, the "sidebar" colour only applies the column header (was correct in 102).

Feel free to make the appropriate changes if you disagree.

(In reply to MM from comment #4)

Message header and quick filter bar and their text can't be styled at all any more (was stylable in 102).

This is bug 1828322.

I think there is no point worrying about 102 here. For SN the UI was completely changed, so a 114/115 fix will likely not be backported.

That is not what I was trying to say. I responded to:

Marking this as regression although the TB 102 behaviour also wasn't correct.

In order to know where something broke, a regression window is really helpful. You marked this as a Supernova 3pane regression, but at the same time state that it was broken in 102 already. That makes it very difficult for us to tackle this bug.

In order to know where something broke, a regression window is really helpful.

Sorry to repeat:

Message header and quick filter bar and their text can't be styled at all any more (was stylable in 102): Bug 1828322, already a SN regression.
The new unified toolbar has the wrong colours (had different colours in 102, certainly not the "frame" colour): The unified toolbar is new in SN, so a SN regression.
The tread tree can't be styled any more, the "sidebar" colour only applies the column header (was correct in 102): The thread pane was re-implemented in SN, so a SN regression.

All broken by SN. When these three regressions are fixed and the functionality is returned back to the inconsistently yet working 102 functionality, I'm happy to file another bug about the remaining inconsistencies. Is that acceptable?

See Also: → 1828322

Richard, would you be able to take a look at this and check what we're missing to make those areas compatible with themes? (if we still have issues, since I think some of those we fixed).

Flags: needinfo?(richard.marti)
Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached file test-webext-theme.xpi

This is a test theme with crazy colours to see what changes. Some colour combinations aren't accessibility conform, don't look at this issues.

Flags: needinfo?(richard.marti)
Target Milestone: --- → 116 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/d81e34b90497
Tweak some third party WE theme issues. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

Comment on attachment 9337361 [details]
Bug 1830609 - Tweak some third party WE theme issues. r=aleca

[Approval Request Comment]
User impact if declined: some not applied parts in UI and not so easy uplifting
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): should be low

Attachment #9337361 - Flags: approval-comm-beta?

Comment on attachment 9337361 [details]
Bug 1830609 - Tweak some third party WE theme issues. r=aleca

[Triage Comment]
Approved for beta

Attachment #9337361 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: