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

fix playerAutopauseWhenSwitchingTabs when minimizing browser #2386

Merged
merged 2 commits into from
Jun 18, 2024
Merged

fix playerAutopauseWhenSwitchingTabs when minimizing browser #2386

merged 2 commits into from
Jun 18, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Jun 16, 2024

Chrome sends two blur events and breaks restoring play EDIT: ... and this patch doesnt didnt fix this :D It does now!

For a hot minute I though above patch fixed this, then that maybe this #2375 did it as I couldnt reproduce the bug anymore, but NO! I was missing the crucial piece. Problem happens only with more than one Browser window! Its not Chrome sending two blur events, its still our background.js :] :|

We handle normal tab switching

chrome.tabs.onActivated.addListener(function (activeInfo) {

but also send blur on Window minimize
chrome.windows.onFocusChanged.addListener(function (wId) {

without check if the window being minimized is the Active one.

I was triggering this bug by using undocked DevTools console = second Chrome window. Now that I understand it I can reproduce it using Chrome with two normal windows. Minimize one after another and two blur events are send to our Tab setting this.played_before_blur first to True just to overwrite it with false on second one.

Two ways to fix this. lazy one is to keep state of focus in ImprovedTube, proper to track focus state in background.js and make sure we only send one blur/focus message. Second option is implemented in 9e76934

Chrome sends two blur events and breaks restoring play
@raszpl raszpl marked this pull request as draft June 17, 2024 07:12
@raszpl raszpl marked this pull request as ready for review June 17, 2024 08:54
@raszpl
Copy link
Contributor Author

raszpl commented Jun 17, 2024

Even found a bug about this #1103 caused by chrome.windows.onFocusChanged
with someone complaining about videos pausing when switching active Window. An option is to distinguish between switching tabs and windows by sending different messages. 'Quality without focus' needs both while playerAutopauseWhenSwitchingTabs should only listen to Tab ones.

@raszpl raszpl marked this pull request as draft June 17, 2024 09:24
@ImprovedTube
Copy link
Member

Two ways to fix this.

what about both working together? (universal question)

Even found a bug about this #1103 caused by chrome.windows.onFocusChanged
with someone complaining about videos pausing when switching active Window. An option is to distinguish between switching tabs and windows by sending different messages. 'Quality without focus' needs both while playerAutopauseWhenSwitchingTabs should only listen to Tab ones.

looking forward!

@raszpl raszpl marked this pull request as ready for review June 18, 2024 03:52
@raszpl
Copy link
Contributor Author

raszpl commented Jun 18, 2024

Patch can go in as is for now. This will fix double blur events on window switch.
Specific fix for #1103 can come later. It will require modifying

} else if (message.focus === true && ImprovedTube.elements.player) {
ImprovedTube.focus = true;
ImprovedTube.pageOnFocus();
} else if (message.blur === true && ImprovedTube.elements.player) {
ImprovedTube.focus = false;
ImprovedTube.pageOnFocus();
document.dispatchEvent(new CustomEvent('improvedtube-blur'));

or
ImprovedTube.pageOnFocus = function () {
ImprovedTube.playerAutopauseWhenSwitchingTabs();
ImprovedTube.playerAutoPip();
ImprovedTube.playerQualityWithoutFocus();
};

and

youtube/background.js

Lines 208 to 220 in 85bf124

chrome.windows.onFocusChanged.addListener(function (wId) {
windowId = wId;
//console.log('onFocusChanged', windowId, tabPrev, tab);
tabPrune(function () {
if (windowId != tab.windowId && tab.tabId && tabConnected[tab.tabId]) {
chrome.tabs.sendMessage(tab.tabId, {action: 'blur'});
//console.log('blur', tab.tabId, windowId);
} else if (windowId && tab.tabId && tabConnected[tab.tabId]) {
chrome.tabs.sendMessage(tab.tabId, {action: 'focus'});
//console.log('focus', tab.tabId, windowId);
}
});
});

so only playerQualityWithoutFocus runs on Window switch.

@ImprovedTube ImprovedTube merged commit 13178a9 into code-charity:master Jun 18, 2024
1 check passed
@raszpl raszpl deleted the patch-16 branch June 18, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants