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

Autoplay fix & more #1719

Merged
merged 7 commits into from
Jul 31, 2023
Merged

Autoplay fix & more #1719

merged 7 commits into from
Jul 31, 2023

Conversation

dodieboy
Copy link
Member

I tried to fix disable autoplay play for a few mili second before pause when video is open on new tab. On my computer, it is fixed but not sure will it work for slower device, so i also did some optimization for startup code hoping to fix the problem.

This PR also update hide detail button svg code

This should fix disable autoplay still play for a milisecond before the video pause when video is open using new tab.

code-charity#1703
childHandler will skip all the check for iron-iconset-svg element now as we do not have feature that need it for now. which will fix a bit of lag when open youtube on new tab
added yt-icon-shape
@ImprovedTube ImprovedTube merged commit b284032 into code-charity:master Jul 31, 2023
@@ -150,6 +154,7 @@ ImprovedTube.ytElementsHandler = function (node) {
} else if (id === 'movie_player') {
if (!this.elements.player) {
ImprovedTube.elements.player = node;
ImprovedTube.elements.player.stopVideo();
Copy link
Member

@ImprovedTube ImprovedTube Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? /worth documenting (@dodieboy)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When youtube first open, too many things is loading. From youtube default code to our loop code. Therefore, the pause code sometimes will lag which cause the few milisecond play time. That what I feel

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I stop the video from loading first until everything is ready

Copy link
Member

@ImprovedTube ImprovedTube Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, my bad! (it is just like @dodieboy said already & the first commit does nothing else)
So not to delay playback for everybody else, it should be conditional to ImprovedTube.storage.player_autoplay === false.

html[it-hide-download-button='hidden'] #menu button:has(svg path[d^="M17 18V19H6V18H17ZM16.5 11.4"]),
html[it-hide-download-button='hidden'] #flexible-item-buttons button:has(svg path[d^="M17 18V19H6V18H17ZM16.5 11.4"]) {
html[it-hide-download-button='hidden'] #menu button:has(svg path[d^="M17 18v1H6v-1h11zm-.5-6.6-.7-.7-3.8 3.7V4h-1v10.4l-3.8-3.8-.7.7"]),
html[it-hide-download-button='hidden'] #flexible-item-buttons button:has(svg path[d^="M17 18v1H6v-1h11zm-.5-6.6-.7-.7-3.8 3.7V4h-1v10.4l-3.8-3.8-.7.7"]) {
Copy link
Member

@ImprovedTube ImprovedTube Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the matches can be shorter predicting minor change. Even a few chars are unlikely to exist twice (and if ever, that's probably better than the feature failing.) (@dodieboy )

@ImprovedTube
Copy link
Member

thanks a lot! relieved to see you again!

@ImprovedTube
Copy link
Member

ImprovedTube commented Jul 31, 2023

uploading to the stores.
(expecting no increase in uninstalls )

@PoorChameleon
Copy link

PoorChameleon commented Aug 2, 2023

Can we have another solution for the autoplay? With autoplay on this causes the player to fade to black from a preview icon into suddenly showing video.

Essentially it causes two transitions, including one that's really jarring instead of just one that goes from icon to video.

Possibly something like mentioned in the comment above

Since it essentially resets the player it also causes issues with timestamps not working #1724

@raszpl
Copy link
Contributor

raszpl commented Aug 5, 2023

while ImprovedTube.elements.player.stopVideo() does fix split second play I feel its too brutal.
maybe patching https://github.com/dodieboy/YouTube-Extension/blob/3e1b34e35e73743161100f06ce6d6e2cdb466060/js%26css/web-accessible/mutations.js#L16 similar to the way we handle codecs https://github.com/dodieboy/YouTube-Extension/blob/3e1b34e35e73743161100f06ce6d6e2cdb466060/js%26css/web-accessible/core.js#L75 would work better by preventing .play() in the first place if autoplay is disabled

if (node.nodeName !== 'SCRIPT' && node.nodeName !== 'iron-iconset-svg' && node.nodeName !== 'svg' && node.nodeName !== '#text'&& node.nodeName !== '#comment' && node.nodeName !== 'SPAN' && node.nodeName !== 'DOM-IF' && node.nodeName !== 'DOM-REPEAT') {
var children = node.children;
this.ytElementsHandler(node);
if (node.nodeName === 'SCRIPT' || node.nodeName === 'iron-iconset-svg' || node.nodeName === 'svg' || node.nodeName === '#text' || node.nodeName === '#comment' || node.nodeName === 'SPAN' || node.nodeName === 'DOM-IF' || node.nodeName === 'DOM-REPEAT') {
Copy link
Contributor

@raszpl raszpl Aug 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in 10aef10 is pure semantics and doesnt do anything. Browser js engine will optimize it to same logical form when loading the page. Would made sense if this was assembly, basic, pascal, even C somewhere in mid nineties with bad compilers, but nowadays its all automagically optimized away.

Copy link
Member

@ImprovedTube ImprovedTube Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes (!== .. && vs === .. || just improves readability, might saves 2 nanoseconds per cycle) )

yet of course childHandler is critical.

everything

besides if (node.getAttribute('itemprop')) ) { ...

@ImprovedTube
Copy link
Member

assuming this is best for today: f6d277b
hopefully, if users of autoplay:off noticed an improvement yet, then one might also open an issue (yet not guaranteed - possible that maany people notice something while nobody opens an issue soon)

wish🤫the project would/could get you guys' attention even more 😅😅🤔,
so everything would be flawless 🐱‍🏍 @raszpl @dodieboy @PoorChameleon

@dodieboy dodieboy deleted the autoplay-fix branch August 12, 2023 14:14
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.

4 participants