-
Notifications
You must be signed in to change notification settings - Fork 218
improve default playback quality of YouTube videos to 1440p (or vq=
, quality=
query-string parameter) (fixes issue #1051)
#1052
Conversation
I tried with this video but it still selects 380 or 480p by default: https://www.youtube.com/watch?v=sPyAQQklc1s Can you check it? |
@MortimerGoro I fiddled with it a bit. fixing now, should have it in a good state by tomorrow. will ping you when it's ready to test again. thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, the hard part will be to maintain it when YouTube breaks it.
quite possibly, yes. still tinkering here and there. I think I can cut the code down considerably. another thought: we could avoid having to roll out new releases but possibly remotely hosting the CSS + JS so if (I mean when) things break on YouTube (though the selectors + DOM structure for changing the Quality haven't changed in years to my knowledge [besides the |
Since we don’t sign the extensions remote hosting would be an exploit vector. |
yeah, I agree. CSP in the |
app/src/common/shared/org/mozilla/vrbrowser/browser/SessionStore.java
Outdated
Show resolved
Hide resolved
fdeb64c
to
dac2b13
Compare
Web extensions are only intended for internal use right now in GV so there aren’t security measures by design I think. |
Thanks for checking! Nope, unrelated - we can probably close that. I haven't found it necessary. Yep, will be handled. I will reference the issue in the commit message when I push it to this PR branch. No, but we could eventually in that issue. VR180 support will require a lot more investigation and likely coordination w/ other browser vendors. |
a94be9b
to
7debaaf
Compare
@MortimerGoro ready for another review, if you have a min |
I just tested this patch with https://www.youtube.com/watch?v=hFjzFSidX3s&mozVideoProjection=360_auto and it default to 360p resolution. |
I see this error in the console
|
If fixed the SyntaxError but it still doesn't work. @cvan I see that you have changed the code to use player.setPlaybackQuality(). I already tried that the first time we added the webextension and added some comments there. I found that
So I think the only way would be to try the fake touches again |
Yeah, I can actually remove the But what I discovered is that you can use I've been able to run my branch successfully on YouTube videos on desktop Firefox + Chrome, and the default minimum quality is changed or if specified by the Where I'm stuck on:
To overcome these issues, I think I'm going to have to stop relying on these proprietary YouTube events to know when we are ready to set the quality. I'll likely have to use a If anyone has any better approaches here, let me know. As you can imagine, the development process of making changes with Android Studio -> Oculus Go every time there's a JS change is slow and difficult to remotely debug via desktop Firefox's Earlier, I did some experimenting with injecting a script hosted elsewhere so we'd be able to update the content script for YouTube whenever without having to ship a new release with the hardcoded content script. @bluemarvin expressed concerns that because GeckoView's WebExtension API is not intended for public/end-developer consumption yet, it's likely unreasonable or irresponsible to inject a third-party script (even if it's ours and hosted on a Mozilla-owned domain). (CSP policies can be enforced, but still that's a possible MITM attack vector.) The precedent I've seen in Gecko, specifically for Android WebCompat issues, is a hardcoded extension that's stored in To speed up development, I've been writing the code and running it from Writing the hacks has been fun, and I still have the old code for manually pressing the buttons in the DOM. So we could go back to that if the This patch is in a broken state right now because I've rewritten it so many times. In the meantime, in between coordinating the v1.1.4 release and other work tasks, I'm going to prioritise working on this to get it consistently stable and ready to review + land. If anyone has recommendations on the above items, I'd appreciate the help and will adjust the patch accordingly. Thanks, everyone. |
I think |
just tried |
f581fe1
to
dd920b4
Compare
I've updated the PR to use the methods discussed. Please give it a try. |
It works. I'm going to merge this for the release |
thanks for the assists! much appreciated ❤️ |
for issue #1051