skhzhang / time-based-themes

Automatically change Firefox's theme based on the time
MIT License
121 stars 13 forks source link

System Theme Detection doesn't work in Firefox #43

Open kartonrad opened 1 year ago

kartonrad commented 1 year ago

Description In Firefox, changes to the System Theme do not reach the extension pages. grafik Even though:

This behaviour seems to be very related to issues the darkreader team had: https://github.com/darkreader/darkreader/issues/9582 https://github.com/darkreader/darkreader/commit/29e9312d7ec81f5cbda19e9952856a00cd465912

Their fix seems to be to collect the messages: matchMedia('(prefers-color-scheme: dark)').addEventListener("change", ({matches}) => console.log(matches)) from all tabs, and then manage the resulting mayhem by filtering duplicate messages.

i can't say i've been able to fully parse their code into my brain, it's quite a lot. the core mechanism of matching media queries seems to be the only way though, and it's unreliability must just be accounted for.

System Information

Steps To Reproduce Steps to reproduce the behaviour:

  1. Open the extention tab, and execute matchMedia("(prefers-color-scheme: dark)").matches
  2. Always returns true. Always.

Expected behaviour It should return false in System Light Mode

kartonrad commented 1 year ago

Interestingly, the system theme doesn't reach the DevTool either. (i have it set to automatic - it is dark on every page)

For some pages ff just assumes dark, and never changes the theme - what a strange issue on their part cus i have "light" specified everywhere that is remotely user facing

kierandrewett commented 1 year ago

Using the prefixed (-moz-system-dark-theme) media query is one way of checking if the OS theme is dark or not, however it only seems to be allowed for privileged pages like about:preferences or in the actual browser.

image image

Not sure if it would be possible to use this in a WebExtension context though.

kartonrad commented 1 year ago

Doesn't work :( image

The listener didnt fire going from dark to light to dark And the direct query always returned false

kierandrewett commented 1 year ago

Did you run this from the browser toolbox?

kartonrad commented 1 year ago

On the bright side however that means my annoying-to-implement workaround wasn't invain

kartonrad commented 1 year ago

Did you run this from the browser toolbox?

I navigated to about:debugging, and opened the extension inspector image

it's where the extension background script lives

kartonrad commented 1 year ago

my workaround has the disadvantage that it only works immediately if you are fcosing on a tab, that is not protected by firefox. otherwise none of the contentScripts get the matchMedia change event - so you have to open a website, reload it, and focus the window then it will update

but ig if you're not lurking on about:newtab, it kinda works

kartonrad commented 1 year ago

and if there was a better one, i figure the dark reader folks would've caught on

kierandrewett commented 1 year ago

my workaround has the disadvantage that it only works immediately if you are fcosing on a tab, that is not protected by firefox. otherwise none of the contentScripts get the matchMedia change event - so you have to open a website, reload it, and focus the window then it will update

but ig if you're not lurking on about:newtab, it kinda works

I mean, if you've just got the new tab page open, you're probably not really using the web browser anyway so I don't see this as too big of an issue tbh. It seems to be working fine for me on Linux.

kartonrad commented 1 year ago

My patch? If so i'm glad!

It probably will have broken the "only on startup thing"

Something another global variable could fix But i didnt want to make too many changes

kierandrewett commented 1 year ago

My patch? If so i'm glad!

It probably will have broken the "only on startup thing"

Something another global variable could fix But i didnt want to make too many changes

Yeah, your patch works really well. I don't see why it needs to send the window.location.href surely just the current color-scheme is sufficient?

kartonrad commented 1 year ago

Yeah just for logging

easonwong-de commented 1 year ago

and if there was a better one, i figure the dark reader folks would’ve caught on

Sad thing is, Dark Reader only needs to run on unprotected tabs. So, matchMedia("(prefers-color-scheme: dark)").matches always works for it.

Maybe we can refer to #31

easonwong-de commented 1 year ago

It seems like Firefox has been struggling to deal with system colour theme for a while. Like, in this repo, “system theme detection” issue has come out and been fixed for some times already.

woj-tek commented 1 year ago

It seems like Firefox has been struggling to deal with system colour theme for a while. Like, in this repo, “system theme detection” issue has come out and been fixed for some times already.

Hmm... in Firefox (on macOS) native "system theme" seems to work fine, same goes for page colours.

I just started using this theme and it seems great - it changed theme to the dark flavour last night but today it got stuck on dark one. For now I set fixed times (that maches my system) as a workaround and this seems to work...