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 reverting #2062, fixes #1342 #1652 #1904 #2389

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Jun 17, 2024

reverting implementation of #2061 , bad idea, YT internally uses same parameter markers so we cant really distinguish between YT resuming and someone opening link with timestamp

this.elements.player.seekTo(0) rewinds, but with side effects of triggering play()

switch from this.elements.player.seekTo(0) to this.elements.video.currentTime = 0 didnt work. YT player actually reads currentTime and will hang while hammering currentTime(whatever) if it doesnt get what it wants :0. Back to seekTo(0) with optional check for paused.

Edit: further testing shows

  • on this link https://www.youtube.com/watch?v=mrbFpLwLQAk&t=568s YT executes video.play() 3 times in FF and 3-5 times in Chrome!
  • first YT executed .play() and seek using currentTime(x) happens before extension manages to even load storage some of the time, but not always - this might be the source of randomly not working. If we are slow enough forcedPlayVideoFromTheBeginning works, if we load fast and our forcedPlayVideoFromTheBeginning() is executed too early YT will happily read currentTime, see its not what it set and set it again.
  • YT really reaaaly wants that seek, if I shimm currentTime:
Object.defineProperty(HTMLMediaElement.prototype, 'currentTime', {
	enumerable: true,
	configurable: true,
	get: function() {
		return ImprovedTube.HTMLMediaElement.get.apply(this, arguments);
	},
	set: function(newValue) {
		console.log('early set', newValue);
		if (newValue != 0) {
			this.currentTime = 0;
			ImprovedTube.HTMLMediaElement.set.apply(this, [0]);
		}
	}
});

video wont play at all and console shows

17:55:42.780 core.js:76 early HTMLMediaElement.prototype.play!!!!!!!!
17:55:43.870 core.js:88 early set 568
17:55:43.871 core.js:88 early set 0
17:55:47.163 core.js:88 early set 568
17:55:47.163 core.js:88 early set 0
17:55:53.249 core.js:88 early set 568.001
17:55:53.249 core.js:88 early set 0
17:55:57.360 core.js:76 early HTMLMediaElement.prototype.play!!!!!!!!
17:55:57.435 core.js:88 early set 568
17:55:57.435 core.js:88 early set 0
17:56:06.608 core.js:76 early HTMLMediaElement.prototype.play!!!!!!!!

YT knows something is wrong and keeps trying :(

To make forcedPlayVideoFromTheBeginning reliable it would have to either trigger way late causing annoying rewind every time, or finally make autoplayDisable reliably stop without playing a single frame. Second option seems better.

@ImprovedTube
Copy link
Member

ImprovedTube commented Jun 17, 2024

hi! @raszpl

can start with if (this.storage.forced_play_video_from_the_beginning , paused= inside

#2061

why not check !this.video_url.match(this.regex.video_time) ?

@ImprovedTube ImprovedTube merged commit 33fb5bc into code-charity:master Jun 17, 2024
1 check passed
@raszpl raszpl deleted the patch-18 branch June 17, 2024 18:45
@raszpl
Copy link
Contributor Author

raszpl commented Jun 17, 2024

can start with if (this.storage.forced_play_video_from_the_beginning , paused= inside

variable declarations on top of function are more readable, reading video?.paused is free, declaring empty let paused just to fill it inside condition is more typing :)

why not check !this.video_url.match(this.regex.video_time) ?

because "reverting implementation of #2061 , bad idea, YT internally uses same parameter markers so we cant really distinguish between YT resuming and someone opening link with timestamp"

We can either respect forced_play_video_from_the_beginning on all videos or users will complain its not working in some situations. It still fails sometime as explained in the Edit :( the later its being executed the more reliable it is, but at the cost of annoying jump if Autoplay is enabled. Bet option is to work on fully reliable autoplayDisable, I already have something almost working.

@ImprovedTube
Copy link
Member

hi @raszpl,
youtube will add the t= parameter to the URL bar? under which circumstance? (then can keep !this.video_url.match(this.regex.video_time), when navigation to get there happened through a link or bookmark.) (and i'm just applying the principle of preparing for later, "hoarding comments & code. While Developer time is our only limit but this project might have double the meanningful code sooner than the "existing half" took #2233 (comment) )
regex

@raszpl
Copy link
Contributor Author

raszpl commented Jun 21, 2024

youtube will add the t= parameter to the URL bar? under which circumstance?

#2061 (comment)

@raszpl
Copy link
Contributor Author

raszpl commented Jun 22, 2024

wait, what is "#t=" ? Where did you saw "#t" usage? # (hash) is used for anchor navigation in links.
Please post code links, those images are to "private-user-images" and often dont render at all :(

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