1387689 - [macOS] Close button is white on heartbeat-style notification bar on current nightly

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[:Gijs (he/him)]

After creating a new profile on nightly yesterday, on the second run or so I got a heartbeat-style "how do you like Firefox" notification bar. But the background of the notification bar was light grey, and the close button at the end was white, which didn't provide enough contrast. Given some of the bugs I've seen go past about close button icon changes, I expect this is a recent photon-related regression.

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1387689 - Set text color on heartbeat notification instead of heartbeat messageText.

This un-breaks the close icon in the notification bar that
picks up the current text color.

Review commit: https://reviewboard.mozilla.org/r/166040/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166040/

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8894881 [details]
Bug 1387689 - Set text color on heartbeat notification instead of heartbeat messageText.

https://reviewboard.mozilla.org/r/166040/#review171288

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5d04dc4ffb9
Set text color on heartbeat notification instead of heartbeat messageText. r=dao

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f5d04dc4ffb9

Comment 5

[ovidiu boca[:Ovidiu]]

Attachment: heartbeat .png

I tested this on Mac OS X 10.12 with Nightly 57.0a1(2017-08-22) and I see that the close button has contrast, but please see the dimensions of the text from the heartbeat, this look smaller compared with the rest of the text from the browser. Please see the attachment.

Comment 6

[Johann Hofmann [:johannh]]

(In reply to ovidiu boca[:Ovidiu] from comment #5)
> Created attachment 8900226 [details]
> heartbeat .png
> 
> I tested this on Mac OS X 10.12 with Nightly 57.0a1(2017-08-22) and I see
> that the close button has contrast, but please see the dimensions of the
> text from the heartbeat, this look smaller compared with the rest of the
> text from the browser. Please see the attachment.

It looks slightly off, but I wouldn't see how that is relevant for this bug. It's highly unlikely that my patch caused that.

Comment 7

[ovidiu boca[:Ovidiu]]

Attachment: Screen Shot 2017-09-14 at 10.28.59 AM.png

I retested this with Nightly 57.0a1(2017-09-13), please see the attachment. 

Mike, I know that you work on Shield, can you please take a look and tell us if this is the correct behavior? 
Thanks

Comment 8

[ovidiu boca[:Ovidiu]]

Attachment: Heartbeat on FF57 vs FF56.png

I did a comparison between FF 57 and FF 56, see the attachment.

Comment 9

[Osmose [:osmose, :mkelly]]

(In reply to ovidiu boca[:Ovidiu] from comment #7)
> Created attachment 8907966 [details]
> Screen Shot 2017-09-14 at 10.28.59 AM.png
> 
> I retested this with Nightly 57.0a1(2017-09-13), please see the attachment. 
> 
> Mike, I know that you work on Shield, can you please take a look and tell us
> if this is the correct behavior? 
> Thanks

Looks like a bug to me! I've filed bug 1400017 to handle it.

Comment 10

[ovidiu boca[:Ovidiu]]

Thanks Mike, based on comment 9 and comment 5 I will mark this verified fixed.