Closed Bug 1301851 Opened 5 years ago Closed 5 years ago
Cannot load the notification message icon for .message
97.82 KB, image/png
58 bytes, text/x-review-board-request
61.91 KB, image/png
1.49 KB, patch
|Details | Diff | Splinter Review|
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 18.104.22.168 (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)
Component: General → Site Identity and Permission Panels
Whiteboard: [fxprivacy] [triage]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Component: Site Identity and Permission Panels → Plug-ins
Product: Firefox → Core
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/26af4399208de016343c799b0dcacc5a732a8d08 Bug 1301851 - Fix the message icons for plugin notifications. r=florian
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
Adrian, it'd be great to have this checked as soon as it's possible for you. Thanks!
The notification icon is too dark, at least on Windows - http://imgur.com/a/jAbnM
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.
(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.
Component: Plug-ins → Theme
Product: Core → Firefox
Target Milestone: mozilla51 → ---
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.
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.
You need to log in before you can comment on or make changes to this bug.