Closed Bug 1407326 Opened 3 years ago Closed 3 years ago

Remove references to invertIconForLightTheme outside of devtools

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

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.
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 ?
Flags: needinfo?(odvarko)
Keywords: dev-doc-needed
Comment on attachment 8917073 [details]
Bug 1407326 - remove references to invertIconForLightTheme in extensions codebase;

https://reviewboard.mozilla.org/r/188086/#review193240
Attachment #8917073 - Flags: review?(lgreco) → review+
> 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.
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).
(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.
Keywords: dev-doc-needed
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f580e00e8f62
remove references to invertIconForLightTheme in extensions codebase;r=rpl
https://hg.mozilla.org/mozilla-central/rev/f580e00e8f62
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
(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
Flags: needinfo?(odvarko)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.