code-charity / youtube

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

popup doesnt resize in Vivaldi / autoplay #2378

Open raszpl opened 5 months ago

raszpl commented 5 months ago

initial popup opens in small window https://github.com/code-charity/youtube/blob/011ea52b9f30ea0c9562967b5bb4697851397448/js%26css/web-accessible/www.youtube.com/player.js#L944-L948 why open small and resize couple of lines later?

https://github.com/code-charity/youtube/blob/011ea52b9f30ea0c9562967b5bb4697851397448/js%26css/web-accessible/www.youtube.com/player.js#L959-L963

lets just open popup at `directories=no,toolbar=no,location=no,menubar=no,status=no,titlebar=no,scrollbars=no,resizable=no,width=${ytPlayer.offsetWidth 0.75},height=${ytPlayer.offsetHeight 0.75}

popup resizing works in Chrome 125 and FF 125, but not in Vivaldi 126 because this very fragile (making bad assumptions) code: https://github.com/code-charity/youtube/blob/011ea52b9f30ea0c9562967b5bb4697851397448/background.js#L255-L260 guesses our just opened popup will be the 'active: true' tab. in Vivaldi at the moment of this call our main browser window is still the active one (UI is slooow, maybe thats the reason). This wrong window has state: 'maximized' and :

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/windows/create#state

A windows.WindowState value. The initial state of the window. The minimized, maximized and, fullscreen states cannot be combined with left, top, width, or height.

means we error out Unchecked runtime.lastError: Invalid value for state so thankfully at least Vivaldi doesnt resize its main window :-)

Also why go to so much trouble in https://github.com/code-charity/youtube/blob/011ea52b9f30ea0c9562967b5bb4697851397448/background.js#L274-L281

when we can append title with https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/windows/create#titlepreface edit: because its not supported by anything :/ https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/windows/update#browser_compatibility

Finally autoplay. &autoplay=${ImprovedTube.storage.player_autoplay_disable ? '0' : '1'} Imho instead playerPopupButton should check if video was playing at the moment of clicking it and pass that state to the popup.

Edit: also found a bug that only shows up in popups /embeds

js&css/web-accessible/www.youtube.com/player.js:993
Uncaught TypeError: player.showControls is not a function

because embeds dont have player controls :)

ImprovedTube commented 5 months ago

why

small in the first moment could make the transition less harsh / more smooth. While it also might changes position & removing URL bar makes the window a bit smaller (can account for that at least)

raszpl commented 5 months ago

Resizing window forces very slow DOM redraw and slows down popup load. I would understand animation, but 1 frame in between is not an animation :) We can instead create new popup window with no url bar directly in background.js. That way there is no need to catch/guess newly created window.open. I just dont understant what this is fixing: https://github.com/code-charity/youtube/blob/011ea52b9f30ea0c9562967b5bb4697851397448/js%26css/web-accessible/www.youtube.com/player.js#L951-L956

ImprovedTube commented 4 months ago

(disclaimer: just adding detail.)

1 frame in between is not an animation :)

1 frame makes a more smooth transition than 0 frames. just reminding of the direction where more change is about to happen

Resizing window forces very slow DOM redraw

contradictory to 1 frame? what to redraw at that point?