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 player.js playerQualityWithoutFocus breaks on switching tabs #2299

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented May 23, 2024

youtube/background.js

Lines 179 to 202 in d806571

chrome.windows.onFocusChanged.addListener(function (windowId) {
chrome.windows.getAll(function (windows) {
for (var i = 0, l = windows.length; i < l; i++) {
if (windows[i].focused === true) {
chrome.tabs.query({
windowId: windows[i].id
}, function (tabs) {
if (tabs) {
for (var j = 0, k = tabs.length; j < k; j++) {
var tab = tabs[j];
if (tab.active) {
chrome.tabs.sendMessage(tab.id, {action: 'focus'});
}
}
}
});
} else {
chrome.tabs.query({windowId: windows[i].id}, function (tabs) {
if (tabs) {
for (var j = 0, k = tabs.length; j < k; j++) {
var tab = tabs[j];
chrome.tabs.sendMessage(tab.id, {action: 'blur'});

sends additional second blur message on tab switch

this

ImprovedTube.qualityBeforeBlur = player.getPlaybackQuality();

sets qualityBeforeBlur again when already minimized, this time to this.storage.player_quality_without_focus. After rocusing back video quality is "restored" to qualityBeforeBlur === this.storage.player_quality_without_focus.

background.js is sending second blur event. Its sending multiple ones when whole browser is minimized for example, or tab switched. Dont know why and if its needed. Definitely sending events to tabs where extension is not running is not good.

@ImprovedTube
Copy link
Member

ImprovedTube commented May 23, 2024

hi @rasz_pl

could also use tab.id 😁

# TAB FOCUS/BLUR


and to undo if the low quality makes it to a new session...? :

if (ImprovedTube.qualityBeforeBlur) { ... } 
else { if ( qualityWithoutFocus === player.getPlaybackQuality() ) {ImprovedTube.playerQuality(ImprovedTube.player_quality || undefined); } ) 

@ImprovedTube ImprovedTube merged commit 8223c7d into code-charity:master May 23, 2024
@raszpl raszpl deleted the patch-8 branch May 23, 2024 12:26
@raszpl
Copy link
Contributor Author

raszpl commented May 23, 2024

hi @rasz_pl

could also use tab.id 😁

should. I dont know why background.js is doing what its doing, but whatever its doing its doing too much of it
to elaborate: why is this code in background.js to begin with? all this mess(double messages, messaging all the tabs) can be replaced by sticking

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

somewhere around here

document.addEventListener('yt-page-data-updated', function (event) {

@ImprovedTube
Copy link
Member

ImprovedTube commented May 24, 2024

hi! @raszpl

idk, previously assumed manifest3 broke it: #1516 (comment)

brings us back here: #1439 and #bugs+#important

version 3.965.zip ( = view repo at october7 2022 )

At least we should still look at 3.965 to fetch the features not migrated to 4.0 (Mixer, Analyzer & Custom CSS & JS)
(and there might be more good code missing since 4.0)

( obvious change in 4.0 stuff was moving stuff from web-accessible youtube-scripts.js to content scripts:

	"content-scripts/extension-context/youtube-features/night-mode/night-mode.js",
	"content-scripts/extension-context/youtube-features/general/general.js",
	"content-scripts/extension-context/youtube-features/appearance/player/player.js",
	"content-scripts/extension-context/youtube-features/appearance/details/details.js",
	"content-scripts/extension-context/youtube-features/appearance/sidebar/sidebar.js",
	"content-scripts/extension-context/youtube-features/appearance/comments/comments.js",					

And there was the plan to allow HTMLmediaelement features on all sites. Or integrate/invite https://github.com/xxxily/h5player / & https://greasyfork.org/en/scripts/30545. ( And care for top URLs like m.youtube & facebook.com/watch) (So that must be why the sub-folders were called "youtube" for 4.0 )

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