1394248 - Update plugins icons in Plugins category/tab in Add-ons Manager/about:addons to Photon specific ones

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: screenshot1.png

Update plugins icons in Plugins category/tab in Add-ons Manager/about:addons to Photon specific ones.

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: screenshot2.png

Comment 2

[Dale Harvey (:daleharvey)]

Sorry Stephen, that time again, we have 2 other plugin icons we use

http://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/plugins/notifyPluginGeneric.png
http://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/plugins/pluginBlocked.png

notifyPluginGeneric we could probably skip updating but pluginBlocked specifically uses the old style so will need a new icon (or can use the current icon but red?)

Comment 3

[Stephen Horlander [:shorlander]]

Attachment: plugin-blocked.svg

Comment 4

[Dale Harvey (:daleharvey)]

Attachment: Bug 1394248 - Update plugins icon.

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

Comment 5

[Dale Harvey (:daleharvey)]

Apologies Stephen, when I looked at actually switching the pluginBlocked icon it seems we do not actually use it at all, will deal with that in https://bugzilla.mozilla.org/show_bug.cgi?id=1348556.

Comment 6

[Dale Harvey (:daleharvey)]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

we do use pluginBlocked-64.png though, will do that icon in this patch as well

Comment 7

[Dale Harvey (:daleharvey)]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187678/diff/1-2/

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

https://reviewboard.mozilla.org/r/187678/#review193176

::: toolkit/themes/shared/non-mac.jar.inc.mn:81
(Diff revision 2)
>  #elif MOZ_SEPARATE_MANIFEST_FOR_THEME_OVERRIDES
>  [extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}] chrome.jar:
>  #endif
>  % override chrome://global/skin/arrow/arrow-lft-hov.gif           chrome://global/skin/arrow/arrow-lft.gif
>  % override chrome://global/skin/arrow/arrow-rit-hov.gif           chrome://global/skin/arrow/arrow-rit.gif
> -% override chrome://mozapps/skin/plugins/notifyPluginCrashed.png          chrome://mozapps/skin/plugins/pluginGeneric-16.png
> +% override chrome://mozapps/skin/plugins/notifyPluginCrashed.png          chrome://mozapps/skin/plugins/pluginGeneric.svg

Overriding a png with an svg doesn't seem like a good idea. Can you update browser-plugins.js to use pluginGeneric.svg directly?

::: toolkit/themes/windows/mozapps/extensions/eula.css:1
(Diff revision 2)
> +

Please don't add this line ;)

Comment 9

[Dale Harvey (:daleharvey)]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187678/diff/2-3/

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

https://reviewboard.mozilla.org/r/187678/#review193190

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4eae12c81330
Update plugins icon. r=dao

Comment 13

[Dale Harvey (:daleharvey)]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

Approval Request Comment
[Feature/Bug causing the regression]: Photon polish
[User impact if declined]: User sees old inconsistent icon
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Nope
[Why is the change risky/not risky?]: icon change with some dead code / asset removal
[String changes made/needed]:

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/4eae12c81330

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8916534 [details]
Bug 1394248 - Update plugins icon.

Photon changes (doesn't seem like just polish ;), Beta57+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/029958b42481

Comment 17

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 58.0a1 (2017-10-11), so I'm marking this bug as VERIFIED.
Thank you very much! \o/