search

ylecuyer / envify

Colorify tabs with current env
https://addons.mozilla.org/en-US/firefox/addon/envify/
40 stars 5 forks source link

Fix: Keep installed theme on non selected tabs. #7 #27

Closed ylecuyer closed 4 years ago

ylecuyer commented 4 years ago

Unfortunately the fix for browser.theme.reset which landed in ff74 isn't enough for this extension. However browser.theme.getCurrent() now behaves correctly so we can save the theme on the extension startup and restore the saved theme instead of resetting it.

To change the theme, the extension needs to be disabled and enabled back.

Smile4ever commented 4 years ago

@ylecuyer What's stopping you from merging this?

ylecuyer commented 4 years ago

I was hoping for someone to test it. As no one complained I think it is working ok for everyone. Did you test it ? Did it work for you ?

I'll make a release at the end of the day with these change

Smile4ever commented 4 years ago

I've tested it. It works for me.

Smile4ever commented 4 years ago

@ylecuyer A small annoyance. Changing the theme back from Dark to Standard when Envify was already installed results in every tab getting black again. getCurrent is not called often enough in this patch to avoid that.

It would be best to add a hook for https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/theme/onUpdated to avoid that.

ylecuyer commented 4 years ago

yes this is what I tried to explain in the issue, you need to disable the extension to change the global theme of the browser.

onUpdated looks handy but there is no way to know if it has been called from another extension, or from envify itself or from the browser style change. So we would be overwriting the theme everytime :/

Smile4ever commented 4 years ago

Why don't you compare the result of getCurrent (cached) with onUpdated, to see whether you should update the cached getCurrent?

ylecuyer commented 4 years ago

I have not idea how to make this work. If you have something working please open a PR and I'll be happy to review it.

For the moment, I'll merge this code and release a new version. People will have to disable the extension when changing their global theme, I'll try to make this clear in the release note

tomprince commented 4 years ago

I can also confirm that the released version is working for me.

ylecuyer commented 4 years ago

thanks for the feeback :+1:

ludsoft commented 4 years ago

Hi, I think I have an bug with this new behavior. The default theme on Firefox 75.0 on Ubuntu 18.04 is not working anymore. I noticed it when two days ago (I guess with the update of envify) instead of having the brown tabs for tabs not in the envify list, I got white ones. i didn't changed any settings, the theme rendered just changed.
Deactivating envify, changing the theme to dark and reactivating the extension works for the dark theme. But going back to the Default one, I get the brown tabs at first, but as soon as I change tabs I get white ones again. Let me ĸnow if I should open a new issue with a bug for this or if I can provide any other information that would help. Thanks.

ylecuyer commented 4 years ago

Hi @ludsoft, Yes please, could you open a new issue and if possible add a screencast or at least screenshots so that we can clearly understand what is going on.