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

shortcut_next_video: null but mouse wheel up results in calling function_name "shortcutNextVideo" #1000

Closed
raszpl opened this issue Jul 27, 2021 · 5 comments
Labels
Bug Bug or required update after YouTube changes 🥳🤩Yay!👏 legendary?

Comments

@raszpl
Copy link
Contributor

raszpl commented Jul 27, 2021

Bug Report:

BUG:

mouse wheel is firing ALL the actions at once that user ever played with but decided to disable. On my install:

On a loaded YT video (autoplay turned off = auto paused) flipping mouse scroll wheel up results in next video being loaded IF you never press any key other than mouse wheel.

HOW:

breakpoint set on https://github.com/code4charity/YouTube-Extension/blob/8b1f01a40fe0cde2a3d9898f08daa9f4e6374648/youtube-scripts.js#L3146

self.storage
.....
shortcut_next_video: null
....

but

        for (var key in self.storage) {
            if (key.indexOf('shortcut_') === 0) {
                var function_name = 'shortcut' + (key.replace(/_?shortcut_?/g, '').replace(/\_/g, '-')).split('-').map(function (element, index) {
                    return element[0].toUpperCase() + element.slice(1);
                }).join(''),
                    data = JSON.parse(self.storage[key]) || {};

                if (
                    (data.keyCode === keys.keyCode || !self.isset(data.keyCode)) &&
                    (data.shiftKey === keys.shiftKey || !self.isset(data.shiftKey)) &&
                    (data.ctrlKey === keys.ctrlKey || !self.isset(data.ctrlKey)) &&
                    (data.altKey === keys.altKey || !self.isset(data.altKey)) &&
                    ((data.wheel > 0) === (wheel > 0) || !self.isset(data.wheel)) &&
                    ((hover === true && (data.wheel > 0) === (wheel > 0) && Object.keys(keys).length === 0 && keys.constructor === Object) || (self.isset(data.key) || self.isset(data.altKey) || self.isset(data.ctrlKey)))
                ) {
                    if (type === 'wheel' && self.isset(data.wheel) || type === 'keys') {
                        event.preventDefault();
                        event.stopPropagation();
                    }

                    ImprovedTube[function_name]();

why?

data
{}
(data.keyCode === keys.keyCode || !self.isset(data.keyCode)) &&
                    (data.shiftKey === keys.shiftKey || !self.isset(data.shiftKey)) &&
                    (data.ctrlKey === keys.ctrlKey || !self.isset(data.ctrlKey)) &&
                    (data.altKey === keys.altKey || !self.isset(data.altKey)) &&
                    ((data.wheel > 0) === (wheel > 0) || !self.isset(data.wheel)) &&
                    ((hover === true && (data.wheel > 0) === (wheel > 0) && Object.keys(keys).length === 0 && keys.constructor === Object) || (self.isset(data.key) || self.isset(data.altKey) || self.isset(data.ctrlKey)))
                
true

so shortcut_next_video: null results in data = {} results in this huge conditional returning true because keys = {}

means we trigger all shortcut_ functions regardless of null variable. Git blame shows this code was reworked 12 days ago 71c89d8#diff-067575607c05a8d0d2e9c27d449a2a51e4d078e2efed6503e0894cee0cb67c48

Was this rework written with assumption self.storage stores ONLY active shortcut_** variables? This would be correct if Shortcuts RESET removed variables from storage, but currently it sets them to null (or "null", probably in even older version).
People with old installs that ever set and then reset a shortcut have one set to null/"null" right now = it will automagically trigger every time you mouse wheel up.

Why only mouse wheel up and not mouse wheel down?

((hover === true && (data.wheel > 0) === (wheel > 0) && Object.keys(keys).length === 0 && keys.constructor === Object) || (self.isset(data.key) || self.isset(data.altKey) || self.isset(data.ctrlKey)))
            ) {

wheel reflects the direction, mouse wheel down is set to 100 and doesnt pass, but down is -100 and automagically pass this conditional with data {} and keys {}

⚬ ImprovedTube Version:
3.339

@raszpl raszpl added Bug Bug or required update after YouTube changes help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) labels Jul 27, 2021
@ghost
Copy link

ghost commented Jul 27, 2021

@ghost ghost closed this as completed Jul 27, 2021
@ImprovedTube
Copy link
Member

never dreamed #1000 would be so special

@raszpl
Copy link
Contributor Author

raszpl commented Jul 27, 2021

whoa you rewrote whole thing? :o I would just fix the conditional and call it done. Nice.

@ImprovedTube ImprovedTube removed the help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) label Dec 28, 2021
@ImprovedTube ImprovedTube reopened this May 3, 2023
@ImprovedTube
Copy link
Member

@ImprovedTube
Copy link
Member

ping :) bump #1565 #2421 @raszpl @victor "@adokseo"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug or required update after YouTube changes 🥳🤩Yay!👏 legendary?
Projects
None yet
Development

No branches or pull requests

2 participants