Closed Bug 1209343 Opened 5 years ago Closed 5 years ago
"This message may be a scam" notification is really hard to read with GTK3 and Ubuntu
4.07 KB, image/png
7.35 KB, image/png
900 bytes, patch
|Details | Diff | Splinter Review|
STR: 1. View a message that Thunderbird thinks is a scam, in Thunderbird Aurora (v43) ACTUAL RESULTS: The scam warning is unreadable (white on light yellow). See attached screenshot. EXPECTED RESULTS: Readable text. NOTE: The "blocked remote content" notification uses a different text-color and is consistent with what I'd expect from using OS-dependent theming, based on bug 1198063. I'm using Ubuntu 15.10 prerelease (currently beta, releasing this month) and Earlybird 43.0a2 (2015-09-28) (thunderbird aurora channel)
For comparison, here's the more-readable other notification, which is rendering as expected per bug 1206516.
I'm guessing we use a special text-color on the scam notification, with the intention of being extra-visible -- but in this case it means we're extra-invisible. :-/
(In reply to Daniel Holbert [:dholbert] from comment #0) > I'm using Ubuntu 15.10 prerelease (currently beta, releasing this month) and > Earlybird 43.0a2 (2015-09-28) (thunderbird aurora channel) I can reproduce this on a different machine running Ubuntu 15.04 as well. (Also: I noticed that I need to tick a checkbox before the scam notifications will show up -- there's a checkbox in the Thunderbird Preferences "Security" pane.)
Requesting tracking, since this is a regression which effectively disables a security feature by rendering its warning unreadable.
This is a toolkit issue. The color is set here: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/notification.css?offset=200#20 . With the corresponding background-image it would be white on red. But with the -moz-appearance: -moz-gtk-info-bar; set here: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/notification.css?offset=200#10 the background-image isn't used. With setting -moz-appearance: none; like on https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/notification.css?offset=200#17 the red would be shown. If you want to go this way I can make a patch.
(In reply to Richard Marti (:Paenglab) from comment #6) > This is a toolkit issue. The color is set here: > https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/ > notification.css?offset=200#20 Thanks for the diagnosis! CC'ing adw, who last touched that line (to fix a similar black-text-on-dark-background bug, bug 1130850). "-moz-appearance:none" does sound like it would work, but it would remove the native look-and-feel which seems unfortunate. Hopefully adw can make a call (or defer to someone who can) about what's the right solution here.
oh, I spoke too soon -- I think I agree that "-moz-appearance: none" is actually the right solution after all. It sounds like this dropdown's background *used* to be drawn (native-looking or not) via the "background" or "background-image" properties, and we were *intentionally* overriding the native background-color in this notification[type="critical"] rule. But now that the native background is drawn via "-moz-appearance" (as a result of bug 1198063's fix), the formerly-overriding "background-image" in this [critical] declaration doesn't end up being visible. So: if I'm understanding correctly, I think I agree with Paenglab that we should add "-moz-appearance:none" to this notification[type="critical"] rule. Hopefully adw can confirm and/or review the patch.
Add the -moz-appearance: none;
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8667495 - Flags: review?(adw)
Comment on attachment 8667495 [details] [diff] [review] Bug1209343.patch Review of attachment 8667495 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, makes sense and works as advertised.
Attachment #8667495 - Flags: review?(adw) → review+
(Tree sheriffs generally require a Try build before they'll land checkin-needed patches. But I'm pretty sure this doesn't need a Try build, so I just went ahead and pushed, to bypass that back-and-forth with sheriffs.) This needs backporting to Aurora 43 [which is where I encountered it, in Thunderbird]. Richard, could you request aurora approval?
Comment on attachment 8667495 [details] [diff] [review] Bug1209343.patch Approval Request Comment [Feature/regressing bug #]: 1187203 [User impact if declined]: Unreadable critical notification bars [Describe test coverage new/current, TreeHerder]: On the way to m-c [Risks and why]: No risk, only CSS one liner [String/UUID change made/needed]: none
Attachment #8667495 - Flags: approval-mozilla-aurora?
Note: - version 42 [current beta] is unaffected, because we disabled GTK3 on that branch (bug 1207310). - version 41 [current release] is unaffected, because GTK3 wasn't turned on until trunk hit version 42 (bug 1186003). Plus, the regressing patch -- bug 1187203 -- didn't land until in Firefox 42.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Comment on attachment 8667495 [details] [diff] [review] Bug1209343.patch OK to uplift to aurora, minor CSS fix.
Attachment #8667495 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.