Closed Bug 990477 Opened 10 years ago Closed 7 years ago

Set widget/button icons for dark theme.

Categories

(Add-on SDK Graveyard :: General, defect, P2)

x86_64
Linux
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: pzhang, Unassigned)

References

(Depends on 1 open bug)

Details

Those Firefox build-in icons, like homepage and bookmark button, can change their styles when dark theme is applied, and smartly change back to normal style when they are moved to menu panel or overflow panel. We can only do this by some kinda dirty hack, we definitely need native support by Addon sdk. The API that I can imagine could be:

   require('ui/button/action').ActionButton({
     id: 'myid', 
     label: 'myLabel',
     icons: {
       'dark': {
         '16': './16-dark.png',
         '32': './32-dark.png'
       },
       'light': {
         '16': './16-light.png',
         '32': './32-light.png'
       }
     }
   });
I don't think we want overcomplicated the API in such way. Imaging for instance what is happening if we implement the similar feature for `ToggleButton`: we should have a set for dark and light, both for unchecked and checked state, for a total of four and a lot of nested objects – where, in SDK, we try to keep the nested object at minimum, `button.icon` and `panel.position` are in fact exceptions, but are just one level.

But we can probably provide some utility functions and events to make easier this process for the devs without any dirty hack. Could you please provide the code you're writing now to make this working? Thanks!
Assignee: nobody → zer0
Flags: needinfo?(pzhang)
First, create widget by CustomizableUI:

  let { Cu } = require('chrome');
  Cu.import('resource:///modules/CustomizableUI.jsm', this);
  CustomizableUI.createWidget({
    id: 'myid',
    label: 'mylabel'
    /* Don't set icon here, or style rules won't work */
  });

and change icon by set a customized attribute (the tricky part):

  function changeIcon(name) {
    require('sdk/window/utils').window.forEach(function(window) {
      window.document.getElementById('myid').setAttribute('data-status', name);
    });
  }

then such style rules will work:

  @-moz-document url("chrome://browser/content/browser.xul") {
        /* styles for button on toolbar with bright theme */
    #myid {
      list-style-image: url("resource://addon-id/addon-name/data/offline-dark.png") !important;
    }

    #myid[data-status="online"] {
      list-style-image: url("resource://addon-id/addon-name/data/online-dark.png") !important;
    }

    /* styles for button on toolbar with dark theme */
    #myid[cui-areatype="toolbar"]:not([overflowedItem=true]):-moz-lwtheme-brighttext {
      list-style-image: url("resource://addon-id/addon-name/data/online-light.png") !important;
    }

    #myid[cui-areatype="toolbar"][data-status="online"]:not([overflowedItem=true]):-moz-lwtheme-brighttext {
      list-style-image: url("resource://addon-id/addon-name/data/online-light.png") !important;
    }

    /* styles for button on menu panel */
    #myid[cui-areatype="menu-panel"],
    toolbarpaletteitem[place="palette"] > #myid {
      list-style-image: url("resource://addon-id/addon-name/data/offline-dark-32.png") !important;
    }

    #myid[cui-areatype="menu-panel"][data-status="online"],
    toolbarpaletteitem[place="palette"] > #myid[data-status="online"] {
      list-style-image: url("resource://addon-id/addon-name/data/online-dark-32.png") !important;
    }
  }

and style sheet is registered by "@mozilla.org/network/io-service;1".
Flags: needinfo?(pzhang)
Ah ok, so you're not using SDK API at all, in this case.

I think we'll probably need a JEP to propose an approach for SDK. You see, it's a bit less trivial than it seems. Because the size of the icon that you set, are both used also for panel and HiDPI display. So for instance, if you set icon for 16, 32 and 64, the "32" icon is both used for toolbar (in HiDPI display) and menu panel (in regular display). So you can't really have a set of icon for "bright" and "dark", because depends by the DPI of your display that would be different.

I don't know how common is this use case to trying to modify high level API for that, plus there is also the edge case where you could have two windows in two different monitors (one HiDPI and one not), or when you move a window from an HiDPI display to a normal ones; and if we implement such behavior the difference will be more noticeable – a dark icon could appears where is supposed to be a light one, or viceversa.

Anyway, I'll bring this bug to the attention of the team and let's see what we can do!

needinfo'ed me as reminder.
Flags: needinfo?(zer0)
Jeff, you said you were going to figure out how important this is.
Flags: needinfo?(jgriffiths)
removing my needinfo here 'cause I bring to the attention of the team; I will keep myself assigned in the meantime.
Flags: needinfo?(zer0)
(In reply to Wes Kocher (:KWierso) from comment #4)
> Jeff, you said you were going to figure out how important this is.

Dark themes ( both lightweight & complete varieties ) seem to be popular:

https://addons.mozilla.org/en-US/firefox/themes/?sort=popular

https://addons.mozilla.org/en-US/firefox/complete-themes/?sort=users

Can someone ( Matteo? ) take a look at those more popular themes and see how ugly our buttons are? Screenshots please.
Flags: needinfo?(jgriffiths) → needinfo?(zer0)
(In reply to Jeff Griffiths (:canuckistani) from comment #6)

> Can someone ( Matteo? ) take a look at those more popular themes and see how
> ugly our buttons are? Screenshots please.

I think there is a misunderstanding. Our buttons are not ugly "per se", they inherits the styles of Firefox. And then, it depends by the icon the developer set: so let's saying a developer set for a button a colored icon: everything is fine, that icon will be displayed in the same way in both themes, you can check with some add-on you have jeff.
But imaging it set a dark icon, like the "home" button in toolbar. Then it switches to dark theme. The dark icon won't be so visible, in fact the "home" button in toolbar changes to a brighter icon.

So it really depends by the custom icon the developers want to display in the buttons, not from our buttons' style.
So far, I didn't see any add-ons that use a different icon for dark and bright theme – most of them are colored – so I don't know how popular this approach is; but to be fair I don't have a lot of add-ons installed.

Plus, as described in comment 3, due the the HiDPI and the fact that icons in panels are the same despite if they're in bright or dark theme – they changes only in toolbar – it's not so straight forward implement in the API something like that, especially if we want to take in account multiple monitors (one in HiDPI and one not).

So probably before invest more time here, we should understand how common is for add-ons using different icons depends if they're in dark or bright theme, once in toolbar – not how popular are the dark themes per se.
Flags: needinfo?(zer0) → needinfo?(jgriffiths)
Hi Pin, 

Can you please provide an example of the problem in action, eg what combination of dark theme and add-on causes the problem, and what the result looks like.

From my perspective I'm aving a hard time with this, as although dark themes are relatively popular amongst all theme users, themes generally are not very popular when you look at the total number of Firefox users.
Flags: needinfo?(jgriffiths) → needinfo?(pzhang)
(In reply to Jeff Griffiths (:canuckistani) from comment #8)
> Hi Pin, 
> 
> Can you please provide an example of the problem in action, eg what
> combination of dark theme and add-on causes the problem, and what the result
> looks like.
> 
> From my perspective I'm aving a hard time with this, as although dark themes
> are relatively popular amongst all theme users, themes generally are not
> very popular when you look at the total number of Firefox users.

Hi Jeff,

We, Mozilla Beijing Office, created some addons for cn users, you can find some here:
   https://addons.mozilla.org/bn-BD/firefox/user/mozillaonline/#my-submissions

some of them will add a toolbar button, like this one: 
   https://addons.mozilla.org/bn-BD/firefox/addon/%E5%BE%AE%E4%BF%A1%E7%BD%91%E9%A1%B5%E7%89%88%E5%8A%A9%E6%89%8B/?src=userprofile

the major colour of the toolbar icon we designed is same as those built-in ones, e.g. home button, download button etc., so the icon will be more like a *feature* of Firefox instead of an *addon* (we are trying to prevent the concept "addon/plugin/extention" when promoting them, because most of the users really don't know the difference), then when user installed a dark theme, the grey icons will be almost *disappeared* which is kinda unacceptable for us.
Flags: needinfo?(pzhang)
Depends on: 992988
What are we going to do with this one?  I think it's fine to close it if other extensions can change the image used with css and js.

Is that possible now Matteo?
Flags: needinfo?(zer0)
Flags: needinfo?(rFobic)
Flags: needinfo?(jgriffiths)
no idea.
Flags: needinfo?(jgriffiths)
(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #10)

> Is that possible now Matteo?

Not using SDK. They have to create their own button. With the SDK we could provide a dirty hack in the documentation; where basically we register a global CSS stylesheet created by a string, where we inject the button's ID. Without bug 992988 there is no really cleaner solution.
Flags: needinfo?(zer0)
I imagine with a firefox dev-edition this is more relevant than it used to. As of solution I think it maybe worth talking to CustomizableUI API designers to see if support for dark / bright themes could be incorporated somehow, that way we could avoid hacks that Matteo was referring to.
Flags: needinfo?(rFobic)
I'm trying to achieve the same thing with a Web Extension. Any hints?
See Also: → 1341893
See Also: 1341893
Since we're going to remove SDK I'm resolving this as wontfix.
Assignee: zer0 → nobody
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.