Closed Bug 1407326 Opened 3 years ago Closed 3 years ago
Remove references to invert
Icon For Light Theme outside of devtools
59 bytes, text/x-review-board-request
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 ?
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f580e00e8f62 remove references to invertIconForLightTheme in extensions codebase;r=rpl
(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
You need to log in before you can comment on or make changes to this bug.