devkral / lightsonplus

fork from lightsOn
39 stars 16 forks source link

Fullscreen detection broken for Firefox #18

Open WildPenquin opened 6 years ago

WildPenquin commented 6 years ago

Hi,

No fullscreen app I've tried (Youtube HTML5 player or Netflix) triggers the fullscreen detection currently. I'm investigating this issue, just to let you know.

This is on Arch/Plasma.

WildPenquin commented 6 years ago

Ok, I believe it breaks here:

if [ ${#activ_win_id} -eq 9 ]; And later: if [ ${#top_win_id} -eq 9 ]; then

So the script assumes window IDs of length 9. But I assure my Firefox window IDs are of length 8. Why is this assumption / check made ?

Changing the check to -le makes the script work for me, but of course it should be first rationalized why this check is made in the first place?

devkral commented 6 years ago

actually it is a fork. I think maybe an attribute was dropped so the check fails. Will look into that

devkral commented 6 years ago

I think it is a strange check if it is really an id. Hope my commit fixes this bug

devkral commented 6 years ago

anyway it just works for X. I see dark for the future. And would more recommend tools like Caffeine.

aufkrawall commented 5 years ago

Note: Class for Firefox requires lower case since Firefox 68. To support both older and newer versions, I think it would be a good idea to use *[Ff]irefox*).

ostaszewskik commented 5 years ago

@aufkrawall just created an pull request #25 . I tested it myself and yours solution ([Ff]irefox) works as expected.