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 slightly more readable autoplayDisable #2157

Merged
merged 4 commits into from
May 6, 2024
Merged

Update player.js slightly more readable autoplayDisable #2157

merged 4 commits into from
May 6, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Apr 3, 2024

No description provided.

@ImprovedTube
Copy link
Member

keeping some references like // quick fix https://github.com/code-charity/youtube/issues/1703 thanks to @AirRaid#9957 (/* document.documentElement.dataset.pageType === "video" */
might be fine at the end of some lines?

i dont know if it is true. if it is true it can prevent somebody from thinking of undoing the change in future.

@raszpl
Copy link
Contributor Author

raszpl commented Apr 4, 2024

I looked into this.

#1703:

also the timing of above function is slightly later through

doesnt appear to be true right now. ImprovedTube.pageType() is reliably called very early in init and later on every page change.

document.addEventListener('yt-navigate-finish', function () {
ImprovedTube.pageType();

but maybe it wasnt there at the time?

AirRaid
And yeah, confirming - it was pageType === "video" and pageType === "channel"

How was this confirmed ? Logging both into console and catching bad case? or just "I changed this and videos pause reliably now so its probably it"? :) because pausing autoplay has been broken ever since #2042 (comment)

Regardless at the end of the day all are same comparison anyway:

document.documentElement.dataset.pageType === "video"
location.href.includes('/watch?')
location.pathname == '/watch'

I would argue using pageType isnt much more readable nor faster. Maybe it should be left alone for just controlling CSS

html[it-header-position=hidden_on_video_page][data-page-type=video] #masthead-container:not([it-search-focus=true]) {

speaking of which
dataset.pageType https://github.com/search?q=repo%3Acode-charity%2Fyoutube+data-page-type&type=code
and it-pathname https://github.com/search?q=repo%3Acode-charity%2Fyoutube+it-pathname&type=code
both are doing same thing, should probably be combined

@ImprovedTube
Copy link
Member

thank you @raszpl

dataset.pageType ...
and it-pathname ...
both are doing same thing, should probably be combined

Yay! Let's document if there is any reason not to keep it simpler (for people who didn't develop extensions)

Extension-context vs. Web-accessible shouldn't distract.

pageType() runs on yt-page-data-updated

ImprovedTube.init = function () {
window.addEventListener('yt-page-data-updated', function () {
ImprovedTube.pageType();
});
ImprovedTube.pageType();

both run on yt-navigate-finish

document.addEventListener('yt-navigate-finish', function () {
ImprovedTube.pageType();

>>> INITIALIZATION
--------------------------------------------------------------*/
extension.features.youtubeHomePage('init');
document.documentElement.setAttribute('it-pathname', location.pathname);
window.addEventListener('yt-navigate-finish', function () {
document.documentElement.setAttribute('it-pathname', location.pathname);

and both should alternatively run on every change of the URL bar

@ImprovedTube
Copy link
Member

  • aand we should notice the other two video-page types: embed, shorts
    ImprovedTube.pageType = function () {
    if (/\/watch\?/.test(location.href)) {
    document.documentElement.dataset.pageType = 'video';
    } else if (location.pathname === '/') {
    document.documentElement.dataset.pageType = 'home';
    } else if (/\/subscriptions\?/.test(location.href)) {
    document.documentElement.dataset.pageType = 'subscriptions';
    } else if (/\/@|(\/(channel|user|c)\/)[^/]+(?!\/videos)/.test(location.href)) {
    document.documentElement.dataset.pageType = 'channel';
    } else {
    document.documentElement.dataset.pageType = 'other';
    }
    };
    ( BTW, if i remember correctly, location.href was/is much faster, even it is a longer string to regex. )

    • and the end of this function (buttons) should also run for embedded videos? (+ #2238 ) ( seems we didnt considered embedded players much yet: https://github.com/search?q=repo%3Acode-charity%2Fyoutube++%28parent.location+OR+embed%2F+OR+%2Fembed%29&type=code )
      ImprovedTube.videoPageUpdate = function () {
      if (document.documentElement.dataset.pageType === 'video') {
      var video_id = this.getParam(new URL(location.href).search.substr(1), 'v');
      if (this.storage.track_watched_videos === true && video_id) {
      ImprovedTube.messages.send({action: 'watched',
      type: 'add',
      id: video_id,
      title: document.title
      });
      }
      this.initialVideoUpdateDone = true;
      ImprovedTube.howLongAgoTheVideoWasUploaded();
      ImprovedTube.dayOfWeek();
      ImprovedTube.channelVideosCount();
      ImprovedTube.upNextAutoplay();
      ImprovedTube.playerAutofullscreen();
      ImprovedTube.playerSize();
      if (this.storage.player_always_repeat === true) { ImprovedTube.playerRepeat(); };
      ImprovedTube.playerScreenshotButton();
      ImprovedTube.playerRepeatButton();
      ImprovedTube.playerRotateButton();
      ImprovedTube.playerPopupButton();
      ImprovedTube.playerFitToWinButton();
      ImprovedTube.playerCinemaModeButton();
      ImprovedTube.playerHamburgerButton();
      ImprovedTube.playerControls();
      }
      };
      • ( This ImprovedTube.videoPageUpdate is called at init and yt-navigate-finish:
        if (ImprovedTube.elements.player && ImprovedTube.elements.player.setPlaybackRate) {
        ImprovedTube.videoPageUpdate();
        ImprovedTube.initPlayer();
        }
        )

@ImprovedTube ImprovedTube merged commit 0172a5c into code-charity:master May 6, 2024
@raszpl raszpl deleted the patch-5 branch May 6, 2024 21:27
@raszpl
Copy link
Contributor Author

raszpl commented May 6, 2024

This is cursed!
1 2df3a5e#commitcomment-141736315

2

} else { video.pauseVideo();

wont work, we are inside a play handler, video hasnt started playing yet so there is nothing to stop.

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