code-charity / youtube

[top~1 open YouTube & Video web-extension] Enrich your experience & choice! 🧰100+clever features📌set&forget📌Longest-standing(yet rare&tough alone. Please help/join🧩us👨‍👩‍👧‍👧) ..⋮ {playback|content discovery|player|extra buttons|distractions|related videos|shorts|ads|quality|codec|full tab|full screen}
http://improvedtube.com
Other
3.29k stars 498 forks source link

fix playerAutopauseWhenSwitchingTabs when minimizing browser #2386

Closed raszpl closed 1 week ago

raszpl commented 1 week ago

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 https://github.com/code-charity/youtube/pull/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 https://github.com/code-charity/youtube/blob/85bf12458d10dbc15ab4d8da057da01b5d8e152d/background.js#L191 but also send blur on Window minimize https://github.com/code-charity/youtube/blob/85bf12458d10dbc15ab4d8da057da01b5d8e152d/background.js#L208 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 https://github.com/code-charity/youtube/pull/2386/commits/9e76934091225bf866bc903e63ced7e0c86faff0

raszpl commented 1 week ago

Even found a bug about this https://github.com/code-charity/youtube/issues/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.

ImprovedTube commented 1 week ago

Two ways to fix this.

what about both working together? (universal question)

Even found a bug about this https://github.com/code-charity/youtube/issues/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 commented 1 week ago

Patch can go in as is for now. This will fix double blur events on window switch. Specific fix for https://github.com/code-charity/youtube/issues/1103 can come later. It will require modifying https://github.com/code-charity/youtube/blob/2e850c87a7603ed5bbea779583bb45132acad03e/js%26css/web-accessible/core.js#L379-L388 or https://github.com/code-charity/youtube/blob/2e850c87a7603ed5bbea779583bb45132acad03e/js%26css/web-accessible/functions.js#L287-L291 and https://github.com/code-charity/youtube/blob/85bf12458d10dbc15ab4d8da057da01b5d8e152d/background.js#L208-L220 so only playerQualityWithoutFocus runs on Window switch.