Proposed fix for embedded video issue (#56) #66
Conversation
… due to a conflict with tracking protection. Current versions of Chrome and Firefox seem to work just fine without it.
…on page (it no longer exists), and some minor cleanup.
|
Hello, thanks for this PR. I would like to test the functionality. Could you provide a page with embedded videos that fails with current h264ify and succeeds with this PR? What settings in Chrome and Firefox are you using when you test? |
|
Hey, thanks for taking a look. Here is the test page I used to reproduce the issue on Firefox. It should only need the "standard" level of tracking protection to manifest (the default). Let me know if you have any trouble. |
|
@Wsiegenthaler i just tested the two videos that you posted on your test page and they both got converted to
|
|
Thanks @dnmTX. Good to hear. |
|
Hmm, with the released version of h264ify, I seem to have trouble reproducing the issue on Firefox 69.0.1 64-bit on both Standard and Strict content blocking. How reproducible is this issue? Could you provide some more information on your environment by pasting the info from about:support? @dnmTX Are you able to reproduce the issue (released version of h264ify not working) on the test page? |
|
@erkserkserks works just fine on my end with the latest Slimjet Browser(chromium) and the latest h264ify 1.1.0. I don't use Firefox and can't test it with it at the moment. P.S. And this is what i get when h264ify is disabled,which proves that the extension is working on the test page: |
|
@erkserkserks Sorry, I didn't realize this until just now but it looks like tracking protection was revamped in Firefox 70 which is possibly why you can't reproduce it with 69. I'm on Nightly which is already at 71 and have been experiencing this issue for at least a couple months. Also, reading through #56 I understand that you and others were able to reproduce a similar (if not the same) issue with Chrome/Chromium. Did I understand correctly? Is that still reproducible? |
|
Ok, I have reproduced the issue on Firefox nightly 71.0a1 (2019-10-03). Turning off cookie blocking causes the released version of h264ify to work correctly. On Firefox beta 70.0b11, I can not reproduce the issue. I'll take another look this PR this weekend when I have some free time. |
|
Any update on this. This version works for me, but master does not. |
|
I think we just needed some time to look things over and make sure there weren't any problems before issuing a release. I've been using it with Firefox since October and nothing's popped up so I feel fairly confident there aren't any issues. @erkserkserks - Let me know if you have any questions or if there is anything I can do to help get this published. |
|
@erkserkserks Any chance we could get a release with this fix? Manually adding this extension every time the browser restarts is getting a bit tiresome. :) Please let me know if you do not plan on merging this and I'll create a Mozilla Add-ons fork for those of us experiencing this issue. |
|
Hi @Wsiegenthaler, interestingly enough, on your test page, I was unable to reproduce the issue on Firefox 75, on Windows and macOS, with both standard and strict enhanced tracking protection. It can be reproduced in Firefox Nightly 77.0a1, and disabling cookie blocking makes h264ify work again. Does this agree with your findings? |
|
Yes, I can confirm that. It's strange that it's not reproducible on 75 but it does explain why so few people have reported it. |
|
@Wsiegenthaler Thanks for confirming. For some reason, this issue seems to occur on nightly Firefox builds but not release builds. I'm hesitant to change the way settings are passed to the extension because it works around an issue that exists/existed. As far as I know, this issue still exists in Chrome: https://stackoverflow.com/questions/3996281/synchronous-message-passing-in-chrome-extensions That said, there's probably a better approach that will accommodate all browsers, I'll need to find time to research and validate. Unfortunately, I am quite busy at the moment. In the meantime, feel free to fork this extension so folks with the same issue have a solution. |
|
That did cross my mind as a possibility though I can't think of a reason why Nightly would differ from release builds (except of course for changes slated for upcoming releases). I'll go ahead and fork the add-on for now and perhaps we can revisit this in a month or two when 76 hits. Thanks again for the awesome work on this extension. It makes a huge difference in my day to day! |
|
For Firefox users having this issue - here is a fork of this extension containing the fix: |
|
Perhaps I miss something, but is 'h264ify-embed-fix' to be installed alongside the h264ify addOn? |
|
@LinuxOnTheDesktop - All you need is h264ify-embed-fix. You can uninstall the original if you still have that. |
|
Works perfectly in firefox |
|
Firefox Nightly here and works great with embedded videos whilst the original never did. |
|
Is this fork still a thing in current Firefox-versions <v110? Or should I use the "original add-on" instead? How can I verify in current Firefox-Builds that the plugin is working correctly? There's no "media" section in the developer debugging section in Firefox. |
yes works fine with 112 both on android and desktop. just use stats for nerds in youtube you can see the codec. |
|
localStorage? function parameters? why not both? |
h264ify currently does not function properly for the majority of embedded videos (see issue #56). It relies on
localStorageto pass user options to the injected script, but using this API is problematic when tracking protection is enabled in Chrome and Firefox. This PR abandonslocalStoragein favor of passing the options into the injected script as a function parameter.This PR also includes some minor unrelated changes:
srcdirectory and movedoptions.jstherevarwithconst, removed unnecessary semicolons, etc)