1407326 - Remove references to invertIconForLightTheme outside of devtools

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

invertIconForLightTheme was removed in Bug 1399886. 

SVG icons for devtools extensions are now automatically inverted/highlighted in the same way as regular devtools icons. Non-SVG icons don't have any kind of filter.
 
There are still a few references to invertIconForLightTheme in the codebase, they should be removed => http://searchfox.org/mozilla-central/search?q=invertIconForLightTheme&case=true&path=

We should look into updating any documentation that might reference this as well. 
And if there is no documentation, we should probably add some.

Comment 1

[Julian Descottes [:jdescottes]]

Adding dev-doc-needed so that we can track adding some guidelines for devtools extentions icons.

Honza: Do you know if this is mentioned anywhere? Is there another documentation entry point for devtools extensions beyond https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Extending_the_developer_tools ?

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1407326 - remove references to invertIconForLightTheme in extensions codebase;

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

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8917073 [details]
Bug 1407326 - remove references to invertIconForLightTheme in extensions codebase;

https://reviewboard.mozilla.org/r/188086/#review193240

Comment 4

[Tim Nguyen :ntim]

> SVG icons for devtools extensions are now automatically inverted/highlighted in the same way as regular devtools icons.

Not quite true, as context-fill is only supported for chrome://, resource:// and mozilla signed extensions. https://dxr.mozilla.org/mozilla-central/rev/tip/layout/svg/SVGContextPaint.cpp#53

> Do you know if this is mentioned anywhere?

I don't think this property was ever exposed to WebExtensions APIs. It's only used internally in the WE code to avoid the icon from being wrongly inverted.

I did expose this property to the add-on SDK in bug 1201718 though, but this shouldn't be a problem as the SDK is dead.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

I confirm that the invertIconForLightTheme wasn't exposed to the addons by the WebExtensions devtools API, and so "removing it" is not going to affeact the existent WebExtensions and it should not require any change to the current docs.

Looking at the fragment linked by :ntim in Comment 4, it seems that (at least currently) only WebExtensions signed internally by Mozilla are going to get an svg icon inverted (anyway it is not different from the current behavior and so I would consider it a separate issue).

Comment 6

[Julian Descottes [:jdescottes]]

(In reply to Tim Nguyen :ntim from comment #4)
> > SVG icons for devtools extensions are now automatically inverted/highlighted in the same way as regular devtools icons.
> 
> Not quite true, as context-fill is only supported for chrome://, resource://
> and mozilla signed extensions.
> https://dxr.mozilla.org/mozilla-central/rev/tip/layout/svg/SVGContextPaint.
> cpp#53
> 

Ah thanks for the info. For some reason I thought we support invertable icons for extensions but it's not the case.
So that means the behaviour is actually exactly the same as today's. Removing dev-doc-needed here.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f580e00e8f62
remove references to invertIconForLightTheme in extensions codebase;r=rpl

Comment 8

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

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

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Julian Descottes [:jdescottes] from comment #1)
> Adding dev-doc-needed so that we can track adding some guidelines for
> devtools extentions icons.
> 
> Honza: Do you know if this is mentioned anywhere?
Hmm..., I don't know about any place.

> Is there another
> documentation entry point for devtools extensions beyond
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/
> Extending_the_developer_tools ?
There is also this repo for examples:
https://github.com/devtools-html/extension-examples

Honza