Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update background.js keep focus/blur messaging to minimum #2375

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Update background.js keep focus/blur messaging to minimum #2375

merged 2 commits into from
Jun 12, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Jun 12, 2024

Keep track of connected tabs. Send only to connected tabs, send only to at most TWO tabs, one losing focus and one gaining it.

I tried experimenting with local

window.addEventListener("blur", (event) => {
});
window.addEventListener("focus", (event) => {
});

as mentioned in #2375 but its much worse, clicking in browser address bar takes focus away and pauses videos :/. chrome.windows.onFocusChanged is only slightly better due to 10 year old Chrome bug https://issues.chromium.org/issues/41116352 :o Chrome doesnt tell us when user alt-tabs away. Firefox does send proper message and reacts to alt-tabbing just fine.

finally fixes #2284 . Tested in Chrome, Vivaldi and FF.

@raszpl raszpl changed the title Update background.js keep focus/blur local Update background.js keep focus/blur messaging to minimum Jun 12, 2024
@raszpl raszpl marked this pull request as ready for review June 12, 2024 12:06
@ImprovedTube ImprovedTube merged commit e8bd44f into code-charity:master Jun 12, 2024
1 check passed
@ImprovedTube
Copy link
Member

(merging untested)

@ImprovedTube
Copy link
Member

thank you so much for caring in depth! @raszpl
(as always)

(Did you check/compare the impact on each connected feature yet?)

chrome.windows.onFocusChanged is only slightly better due to 10 year old Chrome bug https://issues.chromium.org/issues/41116352 :o Chrome doesnt tell us when user alt-tabs away.

Can we make up for that somehow (...?)

have read alt+tab in our bug reports before


window.addEventListener("blur" ... #2375 ... clicking in browser address bar takes focus away and pauses videos

(good to know too. might soon have more auto pause options #2111)

@raszpl raszpl deleted the patch-15 branch June 12, 2024 23:26
@raszpl
Copy link
Contributor Author

raszpl commented Jun 12, 2024

(merging untested)

at least test if its not crashing :-)

(Did you check/compare the impact on each connected feature yet?)

I didnt change the mechanism in the end. Its doing exactly same thing old code was doing, just much less of it. Instead of spamming all unassociated (and hibernated) tabs with messages it now only sends it to YT tabs that send 'tab-connected' first and are still active.

chrome.windows.onFocusChanged is only slightly better due to 10 year old Chrome bug https://issues.chromium.org/issues/41116352 :o Chrome doesnt tell us when user alt-tabs away.

Can we make up for that somehow (...?)

Tried, failed, bug report suggest no one has a workaround. It is what it is.

@raszpl
Copy link
Contributor Author

raszpl commented Jun 13, 2024

Hmm, still getting errors, but now it was one instance

Uncaught (in promise) Error: Could not establish connection. Receiving end does not exist.

after few hours

@raszpl
Copy link
Contributor Author

raszpl commented Jun 15, 2024

at least test if its not crashing :-)

spoilers - it was crashing :] #2380

@ImprovedTube
Copy link
Member

at least test

some more people will already hopefully.
it was also is just an acknowledgement, that the changes make me hurry :) @raszpl

chrome.windows.onFocusChanged is only slightly better due to 10 year old Chrome bug https://issues.chromium.org/issues/41116352 :o Chrome doesnt tell us when user alt-tabs away.

Can we make up for that somehow (...?)

Tried, failed, bug report suggest no one has a workaround. It is what it is.

(could monitor alt+tab keyboard events.)

@raszpl
Copy link
Contributor Author

raszpl commented Jun 22, 2024

Tried, failed, bug report suggest no one has a workaround. It is what it is.

(could monitor alt+tab keyboard events.)

so you didnt click the link and read that bug report :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants