h264ify icon indicating copy to clipboard operation
h264ify copied to clipboard

Proposed fix for embedded video issue (#56)

Open Wsiegenthaler opened this issue 5 years ago • 23 comments

h264ify currently does not function properly for the majority of embedded videos (see issue #56). It relies on localStorage to pass user options to the injected script, but using this API is problematic when tracking protection is enabled in Chrome and Firefox. This PR abandons localStorage in favor of passing the options into the injected script as a function parameter.

This PR also includes some minor unrelated changes:

  • Simplified structure of src directory and moved options.js there
  • Minor code hygiene (replaced var with const, removed unnecessary semicolons, etc)
  • Minor improvements to README

Wsiegenthaler avatar Sep 25 '19 20:09 Wsiegenthaler

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?

erkserkserks avatar Sep 29 '19 21:09 erkserkserks

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 avatar Sep 30 '19 00:09 Wsiegenthaler

@Wsiegenthaler i just tested the two videos that you posted on your test page and they both got converted to avc1 format right away. Using Slimjet Browser(Chromium). Just FYI.

h264ify

dnmTX avatar Sep 30 '19 03:09 dnmTX

Thanks @dnmTX. Good to hear.

Wsiegenthaler avatar Sep 30 '19 15:09 Wsiegenthaler

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 avatar Oct 02 '19 02:10 erkserkserks

@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: h264ify

dnmTX avatar Oct 02 '19 04:10 dnmTX

@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?

Wsiegenthaler avatar Oct 03 '19 02:10 Wsiegenthaler

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.

erkserkserks avatar Oct 04 '19 02:10 erkserkserks

Any update on this. This version works for me, but master does not.

Avi-D-coder avatar Jan 18 '20 22:01 Avi-D-coder

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.

Wsiegenthaler avatar Jan 20 '20 01:01 Wsiegenthaler

@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.

Wsiegenthaler avatar May 03 '20 02:05 Wsiegenthaler

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?

erkserkserks avatar May 03 '20 03:05 erkserkserks

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 avatar May 04 '20 01:05 Wsiegenthaler

@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.

erkserkserks avatar May 04 '20 02:05 erkserkserks

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!

Wsiegenthaler avatar May 04 '20 03:05 Wsiegenthaler

For Firefox users having this issue - here is a fork of this extension containing the fix: https://addons.mozilla.org/en-US/firefox/addon/h264ify-embed-fix

Wsiegenthaler avatar May 04 '20 04:05 Wsiegenthaler

Perhaps I miss something, but is 'h264ify-embed-fix' to be installed alongside the h264ify addOn?

LinuxOnTheDesktop avatar Aug 13 '21 14:08 LinuxOnTheDesktop

@LinuxOnTheDesktop - All you need is h264ify-embed-fix. You can uninstall the original if you still have that.

Wsiegenthaler avatar Aug 15 '21 08:08 Wsiegenthaler

Works perfectly in firefox

shockergit avatar Dec 13 '21 12:12 shockergit

Firefox Nightly here and works great with embedded videos whilst the original never did.

nikallian avatar Jan 09 '23 14:01 nikallian

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.

Krawei avatar Apr 29 '23 20:04 Krawei

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.

shockergit avatar Apr 30 '23 02:04 shockergit

localStorage? function parameters? why not both?

cheater avatar Jul 22 '24 07:07 cheater