Joolee / userchrome-toggle

A Firefox extension to allow a user to toggle custom userchrome styles
31 stars 7 forks source link

Add proper window state handling #6

Open pbaykalov opened 1 year ago

pbaykalov commented 1 year ago

Right now window handling is incomplete: the state of toggles is global but the shortcuts and toolbar acts on the window state and does not change other windows. This manifests itself in the bug: if you try to toggle a switch in two windows, it will take 2 button presses to activate the style in second window.

I do not know about others but it seems to me that per-window handling is desired way of operation. Therefore this pull request makes the behaviour consistent with my idea.

1) Toggles are now explicitly stored with respect to window IDs. 2) Toggle states are reset to "off" upon browser start 3) Toggle states for the new windows are inherited from the last window active.

Please test it yourself, I did not (I have no desire to setup anything to debug it).

pbaykalov commented 1 year ago

In what order are onFocusChanged and onCreated fired when new window is created?

Joolee commented 1 year ago

I don't know about your last question but that shouldn't be too hard to find out. Is it important for the pull-request, e.g. is it compete in the current state?

I'm not using multiple windows myself so I haven't actually thought about this functionality to be honest. I'm willing to merge and publish your code if it works because it looks fine to me (could use some more spaces though 😉) but I don't know when I'll get a chance to test it myself.

pbaykalov commented 1 year ago

Okay I will then try packing it and testing myself.

is it compete in the current state?

If I use getLastFocused() then it won't be a preoblem anymore.

Joolee commented 1 year ago

Packaging is quite easy. Compress the whole thing into a zipfile: https://extensionworkshop.com/documentation/publish/package-your-extension/ Then load it in about:debugging#/runtime/this-firefox

pbaykalov commented 1 year ago

There are multiple problems. 🙃

pbaykalov commented 1 year ago

Seems to be working well now.

pbaykalov commented 1 year ago

The only important thing left is that there is no sense in storing runtime state in local storage, I should just use global variable for that. There is no reason to store runtime state in local storage, right?

pbaykalov commented 1 year ago

fixed it now

pbaykalov commented 1 year ago

Is setting storage is supposed to be serializing the objects?

изображение

So two windows were getting entangled somehow until I used structuredClone.

I tried reproducing this using different structures but this did not happen. So, settings storage is preserving relations in some cases but not others.

изображение