1301851 - Cannot load the notification message icon for .messageImage[value="plugin-hidden"]

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[magicp]

Attachment: Could-not-load-the-image-for-notification-message.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160909030428

Steps to reproduce:

1. Start Nightly with Shockwave Flash 22.0.0.209 (Add-on setting: "Ask to Activate")
2. Go to "http://www.bbc.co.uk/radio1" > notification message will be displayed
3. Check notification message icon 


Actual results:

Cannot load the notification message icon for .messageImage[value="plugin-hidden"]. This because its image url is wrong in browser.css.

Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=65524ee7219ac01b54b89836b67d70ea9d818dbc&tochange=a5a2837e3fed3cfdf6be39fa756e3be7e1257190


Expected results:

- Fix icon image url
- Remove -moz-image-region
- Apply styles same with the other notification icons (e.g. filter, fill, opacity)

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1301851 - Fix the message icons for plugin notifications.

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

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 8790700 [details]
Bug 1301851 - Fix the message icons for plugin notifications.

https://reviewboard.mozilla.org/r/78404/#review76942

Comment 3

[Johann Hofmann [:johannh]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26af4399208de016343c799b0dcacc5a732a8d08
Bug 1301851 - Fix the message icons for plugin notifications. r=florian

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8790700 [details]
Bug 1301851 - Fix the message icons for plugin notifications.

Approval Request Comment
[Feature/regressing bug #]: Bug 1285891
[User impact if declined]: This is a UI regression that prevents the user from easily spotting the difference between vulnerable and non-vulnerable plugins when being asked to enable them. 
[Describe test coverage new/current, TreeHerder]: This is a visual change, so hard to test. I would recommend to wait a day or two until QA has confirmed this on Nightly before uplifting. Should still be in time for Aurora.
[Risks and why]: The fix only involves replacing an image URL. Not likely anything else has been affected.
[String/UUID change made/needed]: None

Comment 5

[Johann Hofmann [:johannh]]

Adrian, it'd be great to have this checked as soon as it's possible for you. Thanks!

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/26af4399208d

Comment 7

[Paul Silaghi, QA [:pauly]]

The notification icon is too dark, at least on Windows - http://imgur.com/a/jAbnM

Comment 8

[magicp]

Attachment: icon-color-is-different-from-expected-one.png

I have confirmed this patch in latest Nightly. Icon color is different from expected one. Please find attached image.

Comment 9

[Adrian Florinescu [:adrian_sv]]

I also did verify this patch in the latest Nightly on Windows XP, Windows7, Windows 8.1, OSX 10.10 and Ubuntu 14.04.

Regarding the color:
- Windows 7 and Windows 10 i can confirm the black color from Comment 7 and Comment 8.
- Windows 8.1, Windows XP and Ubuntu i have a red icon.
- OSX 10.10 i have a white icon.

I assume this is not a concern (referring more to the differences between the Windows versions) for this particular bug, since the color and style matches the other notification icons, but I'm adding the info here since I'm not sure where it's supposed to go.

Comment 10

[Johann Hofmann [:johannh]]

Alright, thanks for testing this quickly and catching that issue. I thought the current color would be a sensible choice on all platforms. We should probably just go with grey for the icon then. I'll follow up. Removing the uplift approval request for now.

Comment 11

[magicp]

(In reply to Adrian Florinescu [:AdrianSV] from comment #9)

> Regarding the color:
> - Windows 7 and Windows 10 i can confirm the black color from Comment 7 and
> Comment 8.
> - Windows 8.1, Windows XP and Ubuntu i have a red icon.
> - OSX 10.10 i have a white icon.

My confirmation is below...

In Firefox 51.0a1 with this patch
- Windows XP, Windows 7, Windows 8.1, Windows 10 and Ubuntu have the black icon. Same with attachment 8791235 [details]
- OSX 10.10 have a white icon with this patch

In Firefox 48.0.2 and 49.0b
- All platform have a gray icon. Same with identity-box icon and attachment 8791235 [details]

If icon color is affected by our environment (e.g. red icon), it is complicated.

Comment 12

[magicp]

(In reply to Johann Hofmann [:johannh] from comment #10)
> Alright, thanks for testing this quickly and catching that issue. I thought
> the current color would be a sensible choice on all platforms. We should
> probably just go with grey for the icon then. I'll follow up. Removing the
> uplift approval request for now.

Hi Johann, I appreciate your effort. Thanks.

Comment 13

[Adrian Florinescu [:adrian_sv]]

Johann, seems that the red color I reported was dependent on the flash version. On older flash versions I got the red icon. Once I updated to the latest version (23), the color of the icon is consistently black.

Apologies for taking this long to figure it out.

Comment 14

[Johann Hofmann [:johannh]]

https://hg.mozilla.org/integration/fx-team/rev/507790d1f7dbc9e349f6787beec4f33f3a435439
Bug 1301851 followup - Change plugin notification icon to grey instead of currentColor. r=me

Comment 15

[Johann Hofmann [:johannh]]

So after trying this out it's probably best to hard code the color to gray. Using black and opacity like the identity block breaks on the different background colors of that notification.

Adrian, no worries. Thanks for letting me know. The red icon is correct for outdated flash versions.

Comment 16

[Johann Hofmann [:johannh]]

Attachment: Patch for uplift

See comment 4 for the uplift request. QA revealed a platform-specific problem with colors that was fixed in a followup, hence a separate patch. Again, fixing this is vital for users to be able to quickly recognize outdated plugins.

Comment 17

[Ritu Kothari (:ritu)]

Comment on attachment 8791758 [details] [diff] [review]
Patch for uplift

Fix was verified on Nightly, Aurora50+

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1fa0170cde41

Comment 19

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/507790d1f7db

Comment 20

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to reproduce this issue on 50.0a2 (2016-09-01), using the STR from comment 0. 
The bug is verified fixed on 
- 50.0b1 build2 (20160920155715)
- latest Aurora 51.0a2 (2016-09-26),
 using
- Windows 10 x64
- Ubuntu 14.04 x86
- Mac OS X 10.11. 
I will set the corresponding flags.