-
-
Notifications
You must be signed in to change notification settings - Fork 563
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
Theme swiching continued, this time working :) #2076
Conversation
* Update themes.js fixing theme switching, it was totally borked for as long as I remember * Update themes.js formatting * Update core.js fixing theme switching * Update core.js found another typo caused by bunched up code * Update core.js rewritten for readability * Update init.js redundant call
yay 👍🏿
what css selector?
otherwise it is only called by shortcut or when changing settings. https://github.com/search?q=repo%3Acode-charity%2Fyoutube%20myColors()&type=code
by not using debugger ;;)
with what side-effect/s?
while 🤔 permanent & shortcut can use the same code / fixes..?
// FEEDBACK WHEN THE USER CHANGES A SETTING (Not needed for pure CSS features, automated already)
always was 😃 (one interpretation: it's white /faint with a grey-green header, kinda like desert, while the designer didn't like the bright 'cartoon colors' from the preview on the whole screen) |
the box with comments, all themes leave it while
shortcut are broken, im fixing them now
this patch fixes that
I combined it with theme switching
no, its spaghetti code. Seriously read Linux Coding Style guidebook https://www.kernel.org/doc/html/v4.10/process/coding-style.html It was designed to minimize errors and ease code review and is used by hundreds of thousands of coders beyond Linux project.
I didnt notice any, but this code as it is now or at some point in the past was copying whole PREF cookie into itself, my PREF cookie was 7 copies when I just looked, and had no F6 at all. Cookie ended with "...000&f5=30030&400" Its a big fail.
Are you sure F6 does anything? like I said I tried setting it manually and reloading YT watch page with no effect on colors in Chrome/Vivaldi.
Its certainly an acquired taste. |
not yet here
👍🏿
where?
long lines !== spaghetti
(we can add another theme and sync previews with the colors to be on YouTube) |
for example
just does this for me
because its doing risky stuff in the catch and not try scope.
|
|
Im gonna correct myself. F6 does indeed work. Its all clear after manually Fixing PREF cookie This is why I was seeing:
By switching Youtube theme with cookie this gets taken care of with YT CSS, but:
so my corrupted cookie was preventing YT from applying dark theme properly |
thank you!
#1565(of course this post is extensive since i'm an extension)
(Maybe we could fix cookies) |
Cookie is regenerated by YT when switching themes. We can totally also fix it when switching our themes. source of cookie bug:
"hl=en&f100=30&f6=400".replace(/(f6=)[^\&]+/, "400") fix is ".replace(/([&]?f6=)[^\&]+/,'')". I suspect this started out as part of bigger code that was also setting cookieValue='' as that would make some sense. and this is how whole cookie thing should be handled in the first place youtube/js&css/web-accessible/www.youtube.com/settings.js Lines 187 to 193 in 967ccbb
|
added setPrefCookieValueByName(name, value) for setting Cookie PREF parameters without manually grepping and messing things up. Called with empty "value" deletes "name" from PREF. Makes sure to preserve & where needed, sanitizes PREF removing broken elements.
switching PREF cookie handling to safe setPrefCookieValueByName()
formatting, youtubeLanguage switched to safe setPrefCookieValueByName() and fixed #2082
Added whole new ImprovedTube.setPrefCookieValueByName = function (name, value) |
Tbh @raszpl i wasnt here for only a week and u did more work then entire extension for month xD |
i think we can test all your changes with many/all users tomorrow. ( btw, this handler & observer optimization attempt is currently undone in the chrome store, because we couldn't be sure, if it increased uninstalls (unless that was just YouTube's big experiment moving video details to the right sidebar) . so i guess i will just undo it again to test your changes alone.) |
fixing theme switching, it was totally borked for as long as I remember
formatting
fixing theme switching
found another typo caused by bunched up code
rewritten for readability
redundant call
added safe setPrefCookieValueByName(name, value) for setting Cookie PREF parameters without manually grepping and messing things up. Called with empty "value" deletes "name" from PREF. Makes sure to preserve & where needed, sanitizes PREF removing broken elements.
switching PREF cookie handling to safe setPrefCookieValueByName()
formatting, youtubeLanguage switched to safe setPrefCookieValueByName() and fixed #2082