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 ClipboardItem (supported in the latest firefox too) #2373

Merged
merged 5 commits into from
Jun 15, 2024
Merged

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Jun 12, 2024

workaround for #2341 by removing clipboard option from Firefox, saving to file works fine

@ImprovedTube
Copy link
Member

only works in chrome?


did you go back to FF from Vivaldi? 🤫

@raszpl
Copy link
Contributor Author

raszpl commented Jun 13, 2024

I use multiple Chrome based browsers (Brave/Vivaldi). Installed FF because it looked like nobody was testing if things work there :) FF testing is super annoying, no way of installing unpacked extension permanently :(

only works in chrome?

details in comment #2341 (comment)
no idea about Safari, Im not going to buy a macbook/iphone just for free extension testing :-)

@raszpl
Copy link
Contributor Author

raszpl commented Jun 15, 2024

ok looks like Safari does support it https://webkit.org/blog/10855/
BUT
"The request to write to the clipboard must be triggered during a user gesture. A call to clipboard.write or clipboard.writeText outside the scope of a user gesture (such as "click" or "touch" event handlers) will result in the immediate rejection of the promise returned by the API call."

no idea why its wrapped in setTimeout

here, maybe someone assumed there is a need to wait for style to take effect and repaint browser window, but that style is not needed in the first place.

also the screenshot icon could use a visual feedback, an CSS transition/animation/brief color change in activation

also navigator.clipboard.write is a promise and will inform us when it fails, for example in Chrome we need explicit focus, otherwise it silently fails. Listening for return with

.then(function () { console.log("ImprovedTube: Screeeeeeenshot tada!"); })
			.catch(function (error) { console.log(error); });

gives us

DOMException: Failed to execute 'write' on 'Clipboard': Document is not focused.

if user happens to click Screenshot icon/trigger shortcut and immediately alt-tabs to another application to paste clipboard. cvs.toBlob is quite slow as it has to encode fullhd png, so probably >100ms before we copy to clipboard, plenty of time for alt tabbing. window.focus(); fixes that.

This might be one of those rare situations where throwing explicit Alert at a user makes sense.

@ImprovedTube
Copy link
Member

visual feedback

used to move / twitch! - didnt come alone that code yet though.

@ImprovedTube ImprovedTube changed the title Update player.js ClipboardItem still not supported in FF Update player.js ClipboardItem (supported in the latest firefox too) Jun 15, 2024
@ImprovedTube ImprovedTube merged commit 85bf124 into code-charity:master Jun 15, 2024
1 check passed
@raszpl raszpl deleted the patch-5 branch June 15, 2024 16:03
@raszpl
Copy link
Contributor Author

raszpl commented Jun 16, 2024

visual feedback

used to move / twitch! - didnt come alone that code yet though.

icon or whole player window? :) I can imagine whole player window glitching due to old code injecting that weird CCS for a split second.
I was thinking of something like:

it-player-button:active > svg,
.improvedtube-player-button:active {
    fill: var(--yt-spec-call-to-action);
    transition: fill 0s;
}
.it-player-button > svg,
.improvedtube-player-button {
    transition: fill 1s;
}

where clicking extension buttons in/under player flashes color for a second. --yt-spec-call-to-action (link color) works in every theme.

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