1475094 - Heartbeat close button invisible on dark theme

Status: RESOLVED FIXED

(Firefox :: Normandy Client, enhancement, P3)

Description

[Paul]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180704192850

Steps to reproduce:

The "Please help improve Firefox by taking this Survey" banner appeared at the top of every tab, with options to Take Survey and Learn More. Not being representative of normal users, I wanted to dismiss it without taking the survey. However, this turned out to be impossible.


Actual results:

There's no X that would dismiss the banner. Moreover, Learn More leads to the instructions at https://wiki.mozilla.org/Firefox/Shield/Heartbeat, which suggest to open about:config and "toggle extensions.shield-recipe-client.enabled to false". There was no extensions.shield-recipe-client.enabled in about:config, and creating a boolean with the value of false had no effect.

Eventually I clicked on Take Survey. This was the opposite of what I wanted to do, but was the only remaining option. Doing so got rid of the banner.


Expected results:

Any intrusive banner should have an X to immediately dismiss it. This is a no-brainer. Yes, it will decrease the number of people answering the survey in the short run, but not including one will annoy people and make them more likely to leave the browser entirely.

Also, the instructions in Learn More should work with immediate effect.

Comment 1

[Stefan [:StefanG_QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0 (20180704003137)

I have tested this report in Ubuntu 18.04 x64 using the latest Nightly and Firefox release build. However, since I'm not selected to participate in the survey I`m not able to test it. As noted in the Wiki I tried to add "extensions.shield-recipe-client.enabled" set to "true" in order to participate in the survey but once I restart the browser, the newly added pref disappeared. 

Please change if this is not the correct component.

@Michael could you let me know how I can participate in the survey in order to test this report properly?

Comment 2

[Osmose [:osmose, :mkelly]]

Redirecting to mythmon, I'm no longer the go-to guy for Shield.

Comment 3

[Michael Cooper [:mythmon]]

There is supposed to be a close button on the far right of the heartbeat prompt. On Nightly, I see the close button. I'll test on release.

The wiki page is out of date, the correct preference is "app.normandy.enabled". I'll update it.

For testing, the Normandy devtools can be used to trigger a heartbeat recipe without being selected for it: https://github.com/mozilla/normandy-devtools. This won't allow the heartbeat to be shown more than once per recipe or per day though.

Comment 4

[Michael Cooper [:mythmon]]

On release 61, I also see the close button. You can summon a Normandy heartbeat prompt at will with the following code in the browser console:

ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.import("resource://normandy/lib/Heartbeat.jsm");
ChromeUtils.import("resource://normandy/lib/SandboxManager.jsm");
new Heartbeat(Services.wm.getMostRecentWindow("navigator:browser"), new SandboxManager(), {flowId: "test", message: "message", learnMoreMessage: "Learn More", learnMoreUrl: "https://mozilla.org"});

Comment 5

[Stefan [:StefanG_QA]]

Thanks for the info [:mythmon].

I have tested this on Win10 x64, Ubuntu 18.04 & 14.06 but I did not manage to reproduce the mentioned behavior. On all tested platforms, I can see the "X" button right next to the "Learn More" link. When the X button is pressed the survey is closed.

Paul, could you provide a short video showing the issue?

Comment 6

[Fede]

I am suffering from the same (I think), you can see it in the following picture:

https://ibb.co/jrATmy

The button is actually there, but it's not visible and, unless you know where to look (which I did after reading the comments on this bug), it looks like you can't dismiss the bar.

This is using Firefox 61.0.1 (64-bit) on latest Mint (19 Tara) fully updated and using the optional 4.15.0-24 kernel, using the KDE desktop and the Breeze Dark desktop theme. The PC is an i5 2500K, Radeon HD 6950 using the radeon kernel module, 4G RAM, in case it affects in any way.

Comment 7

[Michael Cooper [:mythmon]]

Fede: I notice you're using a dark Firefox theme. Is that the built-in dark theme?

Paul: Are you also using a Firefox dark theme?

Comment 8

[Paul]

Yes, I have widget.chrome.allow-gtk-dark-theme=true.

Like Fede, I am also running KDE. With the Kubuntu theme, which is also a dark theme.

I have had no other dark theme bugs in the past few months of using this option.

Comment 9

[Fede]

In my case the pref Paul mentions is deactivated (false), same as for widget.content.allow-gtk-dark-theme, but the dark theme is enabled using the customize tab.

Comment 10

[Michael Cooper [:mythmon]]

That information, along with the screenshot in comment 6, make me pretty confident that the problem here is that the close button reacts to the dark theme, but the background it sits on does not. That makes it a white button on a nearly white background, making it effectively invisible.

Comment 11

[Ethan Glasser-Camp (:glasserc)]

I tried to reproduce this today and was unable to. The close button is produced by toolkit/content/widgets/notificationbox.js, and it seems that the styles for this element come from toolkit/themes/shared/global.inc.css. That CSS hasn't changed since this bug was reported (it uses `fill: currentColor`, although I don't know how that's implemented). So I have neither an explanation for why the bug should have gone away nor any way to find it to investigate further. I know this is unsatisfying Fede, Paul, but do you still see this bug or did it maybe get fixed by someone else at some point? I tried:

- Hamburger menu -> Customize, Themes -> Dark
- Ctrl-Shift-J to open browser console
- Paste code to trigger heartbeat

I'm running on NixOS, so it's possible that there's some difference between how Firefox on Ubuntu is packaged, but it seems kind of like a stretch.

N.B. As of bug 1440778, heartbeat no longer needs a sandbox manager, so you may have to use this code instead:

```
ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.import("resource://normandy/lib/Heartbeat.jsm");
new Heartbeat(Services.wm.getMostRecentWindow("navigator:browser"), {flowId: "test", message: "message", learnMoreMessage: "Learn More", learnMoreUrl: "https://mozilla.org"});
```

Comment 12

[Fede]

Hi Ethan - I just tried pasting those instructions into the console but they did not work:

```
17:52:42.066 ChromeUtils.import("resource://gre/modules/Services.jsm");
17:52:42.076 ReferenceError: ChromeUtils is not defined[Learn More] debugger eval code:1:1
17:52:48.139 ChromeUtils.import("resource://normandy/lib/Heartbeat.jsm");
17:52:48.150 ReferenceError: ChromeUtils is not defined[Learn More] debugger eval code:1:1
17:52:53.949 new Heartbeat(Services.wm.getMostRecentWindow("navigator:browser"), {flowId: "test", message: "message", learnMoreMessage: "Learn More", learnMoreUrl: "https://mozilla.org"});
17:52:53.957 ReferenceError: Heartbeat is not defined[Learn More] debugger eval code:1:1 

```

I just tried opening the console via hotkey (F12), then copying and pasting the instructions one by one (trying all at once makes no difference, other than it only triggers the first error message).

So I am unable to test. Besides, I am no longer using a dark theme, but I wouldn't mind testing if I could bring up the banner again. Do I need a nightly or debug build of Firefox for this?

Comment 13

[Ethan Glasser-Camp (:glasserc)]

Sorry, I guess I should clarify -- I'm not talking about the thing called "Web Console" (which you open with F12 or Ctrl-Shift-K), which lets you debug a specific page/site, but rather the thing called "Browser Console" (under Web Developer -> Browser Console or Ctrl-Shift-J) which lets you debug Firefox itself. The browser console out-of-the-box may not have an input area at first -- you can turn it on by opening the "Web Console", clicking the three-dots menu in the upper right and going to Settings, and then checking "Enable browser chrome and add-on debugging toolboxes". You shouldn't need a nightly or debug build of Firefox. I just did it on my personal laptop which is running Firefox 63 (I think on Ubuntu 18.10).

Comment 14

[Fede]

Hi Ethan,

Apologies for the delay in answering, got the notification on the phone when I was away from the computer and then forgot about it completely ^^U.

I tried on the browser console but still could not make it work. While the first two instructions appeared to do their thing without complaints, the third caused an error:

13:47:55.261 ChromeUtils.import("resource://gre/modules/Services.jsm");
13:47:55.268
NonSyntacticVariablesObject { EXPORTED_SYMBOLS: (1) […], Services: {…}, initTable: undefined, AppConstants: {…}, XPCOMUtils: {…}, … }
13:48:02.735 ChromeUtils.import("resource://normandy/lib/Heartbeat.jsm");
13:48:02.744
NonSyntacticVariablesObject { EXPORTED_SYMBOLS: (1) […], Heartbeat: (), AppConstants: {…}, Preferences: (), Services: {…}, TelemetryController: {…}, setTimeout: (), setTimeoutWithTarget: (), clearTimeout: (), setInterval: (), … }
13:48:13.179 new Heartbeat(Services.wm.getMostRecentWindow("navigator:browser"), {flowId: "test", message: "message", learnMoreMessage: "Learn More", learnMoreUrl: "https://mozilla.org"});
13:48:13.196 TypeError: options is undefined[Learn More] debugger eval code:81:9

This is on Firefox 65.0 on Mint 19 - maybe this version needs slightly different commands?

Comment 15

[Ethan Glasser-Camp (:glasserc)]

Hi Fede, can you try the code that :mythmon posted in comment 4 instead? I think 65 is too early for the bug I posted.

Comment 16

[Fede]

Alright, that worked. I tested four situations:

• Light desktop theme, light Firefox theme
• Light desktop theme, dark Firefox theme
• Dark desktop theme, light Firefox theme
• Dark desktop theme, dark Firefox theme

To summarize, the issue depends on the desktop theme, and not the Firefox theme. With a dark desktop theme (possibly any desktop theme that provides a close button that is light colored?) the close button is indistinguishable from the background, while with a light desktop theme (one that provides a dark close button?) the close button is perfectly visible. To reiterate, the button is always there, it's just practically impossible to spot if using a dark desktop theme.

Comment 17

[Ethan Glasser-Camp (:glasserc)]

OK, that's super interesting. Thank you very much for reporting it.

:Gijs suggested that the notification bar already has support for most of the desktop theme things. The reason that this one doesn't is because we expressly override background color (https://searchfox.org/mozilla-central/source/toolkit/components/normandy/skin/shared/Heartbeat.css#7) and text color (https://searchfox.org/mozilla-central/source/toolkit/components/normandy/skin/shared/Heartbeat.css#52) but not close button color. It sounded like Gijs would prefer that rather than trying to slap more CSS on top of this, we should remove these customizations and rely on the normal notification bar code. What do you think about that, :mythmon?

Comment 18

[Michael Cooper [:mythmon]]

I'm generally in favor of making Heartbeat less weird. My only concern is how the heart icon would look like on a dark background. Lets give it a shot and see how it looks?

Comment 19

[Ethan Glasser-Camp (:glasserc)]

Attachment: Heartbeat dark theme current.png

Comment 20

[Ethan Glasser-Camp (:glasserc)]

Attachment: Heartbeat dark theme proposed.png

This is what it looks like on Nightly when I take out the hard-coded white colors. I think it looks better with a dark theme than the previous, but I'm willing to be contradicted.

Comment 21

[Ethan Glasser-Camp (:glasserc)]

There are three other hard-coded colors here which are the ones used in .text-link (which I think is just the "Learn More" link). I considered taking these colors out too but I guess they have been chosen to match the colors of the stars. I feel like we might have a similar problem to this one day if we get a user who is using B00merang Glass but I don't see a way around it as long as we render stars using SVGs (which, presumably, have hard-coded colors).

I'll send a patch; :mythmon, who should I ask about the visual aesthetics?

Comment 22

[Ethan Glasser-Camp (:glasserc)]

Attachment: Heartbeat light proposed.png

For completeness, here is also what the heartbeat looks like with a "light" theme once I remove the background colors from the heartbeat CSS.

Comment 23

[Ethan Glasser-Camp (:glasserc)]

Attachment: Bug 1475094: remove colors and styles from heartbeat

These hard-coded colors actually remove our ability to respond to
the user's theme. Get rid of them.

Comment 24

[Michael Cooper [:mythmon]]

I'm happy with removing the hard coded links, and allowing heartbeat to match the rest of the UI. I think the closest thing we have to an authority about the aesthetics is the people using it.

Tyler, do you have any opinions about the screenshots that Ethan posted?

Comment 25

[Ethan Glasser-Camp (:glasserc)]

Attachment: Heartbeat dark with plain link style.png

For completeness, this is the version once I have taken the custom link color out.

Comment 26

[Tyler Downer [He/Him]]

That looks great to me. +1

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/688888df6835
remove colors and styles from heartbeat r=Gijs

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/688888df6835