functions.js click handler catch html5-video-container

[raszpl]

html5-video-container instead of html5-main-video. Fixes vertical video click handler

#2042

[ImprovedTube]

Should be optional(? some users might be used to the current way)

[raszpl]

Should be optional(? some users might be used to the current way)

used to how? :-) this is for setting invisible flag for disabling autoplay. Current code is bad as described #2042 (comment)

[raszpl]

in fact its still too much code in the handler, this

youtube/js&css/web-accessible/functions.js

Line 448 in 04a604b

if (ImprovedTube.elements.player && ImprovedTube.elements.player.classList.contains('ad-showing') === false) {

is redundant, we already check for ad-showing in

youtube/js&css/web-accessible/www.youtube.com/player.js

Line 11 in 04a604b

if (!this.user_interacted && video.classList.contains('ad-showing') === false &&

[ImprovedTube]

Currently clicking on the player or play will bypass autoplay-off, while clicking anywhere else wont.
Since we have many users, we cant change any daily behavior, without many people wondering.
Just saying.

[raszpl]

Currently clicking on the player or play will bypass autoplay

"current way" is broken autoplayDisable with extension still autoplaying videos despite autoplayDisable being set to On.

youtube/js&css/web-accessible/functions.js

Lines 454 to 455 in 04a604b

	&& (path[i].className.indexOf('html5-main-video') !== -1
	|| path[i].className.indexOf('ytp-play-button') !== -1)) {

is broken on vertical videos, have to click on the vertical strip with video, meanwhile YT player listens on whole Video element, and finally reading below its broken because things are being initialized too late making it possible for user to click before Extension loaded completely :(

All in all those two handlers (keyboard and mouse) are only used to disable autoplayDisable aka let user play video with with "Force Autoplay Off" option enabled. The only thing users should notice changing is autoplayDisable working correctly :)

---

YT sometimes actually tries to autoPlay video 2-3 times in a row when loading/refreshing page :o This is why every time autoplayDisable fails to properly pause video you can see couple console.log("autoplay:off - should we pause here again?").

Our mousedown

youtube/js&css/web-accessible/functions.js

Line 446 in 04a604b

ImprovedTube.onmousedown = function (event) {

is somehow being loaded and run too late and doesnt catch user clicking Play immediately when page loads. This all stems from Extension being broken into multitude of .js files. It should really be all combined into one .js and one .css file in a build process.

Now Im scratching my head how

youtube/js&css/extension/init.js

Lines 58 to 63 in 65bfc33

	extension.inject([
	'/js&css/web-accessible/core.js',
	'/js&css/web-accessible/functions.js',
	'/js&css/web-accessible/www.youtube.com/appearance.js',
	'/js&css/web-accessible/www.youtube.com/themes.js',
	'/js&css/web-accessible/www.youtube.com/player.js',

autoplayDisable from player.js can be working earlier than mousedown from functions.js hmmm, ah here we go:

autoplayDisable is initialized in

youtube/js&css/web-accessible/init.js

Line 96 in 65bfc33

this.playerOnPlay();

and mousedown lower here

youtube/js&css/web-accessible/init.js

Line 100 in 65bfc33

this.onmousedown();

wrong order, but even switching order doesnt work for me, listener is unable to catch any clicks while while page is loading
and thats because ImprovedTube.elements.player is not initialized yet by

youtube/js&css/web-accessible/functions.js

Lines 173 to 175 in 04a604b

	} else if (id === 'movie_player') {
	if (!this.elements.player) {
	ImprovedTube.elements.player = node;

its timing problems all the way down :/

[ImprovedTube]

YT sometimes actually tries to autoPlay video 2-3 times in a row when loading/refreshing page :o This is why every time autoplayDisable fails to properly pause video you can see couple console.log("autoplay:off - should we pause here again?").

Sounds like you found proof why to undo that! ❤️

It should really be all combined into one .js and one .css file in a build process.

Yes! if it helps.

(Nor sure single files help [this] open development,
since it can also be more engaging and more efficient to work with a long one)

broken on vertical videos, have to click on the vertical strip with video, meanwhile YT player listens on whole Video element.

So we can include the whole/same area.
So that will pause despite a click? ( "#2042 #1809").

( We could call the current autoplayDisabled and a this new one: autoplayDisabled, unless i happen to clicking around anywhere on the page already (which is simpler and will keep working even if the DOM / player changes in future)
)

[raszpl]

youtube/js&css/web-accessible/functions.js

Lines 454 to 455 in 04a604b

	&& (path[i].className.indexOf('html5-main-video') !== -1
	|| path[i].className.indexOf('ytp-play-button') !== -1)) {

is broken on vertical videos

found it, should be html5-video-container instead of html5-main-video, modified the patch. Fixes vertical video click handler, example vertical video https://www.youtube.com/watch?v=brNOkLwYBXc

Sounds like you found proof why to undo that! ❤️

Were there any other cases of people having problem with clicking Play and extension overriding it? having to press Play several times to get video to play?

It should really be all combined into one .js and one .css file in a build process.

Yes! if it helps.

(Nor sure single files help [this] open development, since it can also be more engaging and more efficient to work with a long one)

in Build process - same source files as now in the github repo, but automagically combined into one by build script packaging extension for the release

broken on vertical videos, have to click on the vertical strip with video, meanwhile YT player listens on whole Video element.

So we can include the whole/same area. So that will pause despite a click? #1809

people in that thread describe specifically clicking Play Button and that should have been handled with current code.
Also I dont know if all those people had "autoplay:off" setting activated. It would be really handy to ask people to dump their [HTML] element attributes when reporting bugs, as that contains all the settings already and is somewhat easy for a user to grab. Ideally we would have dedicated button to get settings that is quicker and easier to find than "Export settings" menu, something to push settings into clipboard would be ideal, or something popping a window with all settings in text form to copy.

#1809 (comment):
"So basically: Any testing for this issue will need to cover each of the different "page loading conditions", since the DOM structure differs between them."

This might be right on the money. It can be a case of ImprovedTube.elements.player not being initialized properly in some situations. Maybe its better to use document.querySelector('.html5-video-container') directly in click handlers as a fallback? something like

document.querySelector('.html5-video-container')
		if ((ImprovedTube.elements.player || document.querySelector('.html5-video-container')) && ImprovedTube.elements.player.classList.contains('ad-showing') === false) {

[ImprovedTube]

thanks @raszpl

fallback

please. guess we cant have enough fallbacks when selecting the video.

Were there any other cases of people having problem with clicking Play and extension overriding it? having to press Play several times to get video to play?

maybe not, just #1809

in Build process

(yes, i just also enjoy big files. Not sure who else.)