Closed Bug 1301851 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 51
Iteration:
52.1 - Oct 3
Tracking Status
firefox49 --- unaffected
firefox50 --- verified
firefox51 --- verified

People

(Reporter: magicp.jp, Assigned: johannh)

References

Details

(Keywords: regression, Whiteboard: [fxprivacy] )

Attachments

(4 files)

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)
Blocks: 1285891
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → General
Component: General → Site Identity and Permission Panels
Whiteboard: [fxprivacy] [triage]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Keywords: regression
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Component: Site Identity and Permission Panels → Plug-ins
Product: Firefox → Core
Flags: qe-verify+
Comment on attachment 8790700 [details]
Bug 1301851 - Fix the message icons for plugin notifications.

https://reviewboard.mozilla.org/r/78404/#review76942
Attachment #8790700 - Flags: review?(florian) → review+
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
Attachment #8790700 - Flags: approval-mozilla-aurora?
Adrian, it'd be great to have this checked as soon as it's possible for you. Thanks!
Depends on: 1302570
https://hg.mozilla.org/mozilla-central/rev/26af4399208d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
The notification icon is too dark, at least on Windows - http://imgur.com/a/jAbnM
Flags: needinfo?(jhofmann)
I have confirmed this patch in latest Nightly. Icon color is different from expected one. Please find attached image.
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.
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.
Flags: needinfo?(jhofmann)
Attachment #8790700 - Flags: approval-mozilla-aurora?
(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.
(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.
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.
Flags: needinfo?(jhofmann)
Component: Plug-ins → Theme
Product: Core → Firefox
Target Milestone: mozilla51 → ---
Target Milestone: --- → Firefox 51
https://hg.mozilla.org/integration/fx-team/rev/507790d1f7dbc9e349f6787beec4f33f3a435439
Bug 1301851 followup - Change plugin notification icon to grey instead of currentColor. r=me
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.
Flags: needinfo?(jhofmann)
Attached patch Patch for upliftSplinter Review
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.
Attachment #8791758 - Flags: approval-mozilla-aurora?
Attachment #8791758 - Attachment is patch: true
Comment on attachment 8791758 [details] [diff] [review]
Patch for uplift

Fix was verified on Nightly, Aurora50+
Attachment #8791758 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Iteration: --- → 52.1 - Oct 3
You need to log in before you can comment on or make changes to this bug.