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

Call to requestAnimationFrame causes crash in Firefox browser extensions #438

Closed
NoxHarmonium opened this issue Jul 30, 2019 · 6 comments
Closed

Comments

@NoxHarmonium
Copy link

NoxHarmonium commented Jul 30, 2019

I have a Reagent application that is embedded in a cross browser extension. It is mounted into the DOM of the current tab when it is activated.

The application works fine in Firefox and Chrome when outside of the extension context. It works fine in the extension context with Chrome but in Firefox whenever an event occurs (an atom is reset) after the initial render the following error occurs:

'requestAnimationFrame' called on an object that does not implement interface Window

The React application stops working after this and is effectively dead.

This error isn't shown in the page logs or the background page logs, its actually shown in the browser console under Menu > Web Developer > Browser Console.

I tracked it down to the batching implementation and wrote a fix at:

NoxHarmonium@fc976a8

I branched off v0.8.1 because I was getting unrelated crashes when branching off the current master branch.

It fixes the issue when I build locally and import it into my browser extension.

I would assume that this would be a relatively safe fix. I don't think binding a function to it's own context would cause many issues but I need to test it out nonetheless.

I will also need to create an isolated test project to demonstrate the issue but until then if you want to check out the branch of my project that reproduces the issue check out: https://github.com/agiledigital/mule-preview/tree/attribute-viewer specifically the code that triggers the exception is https://github.com/agiledigital/mule-preview/blob/attribute-viewer/client/src/main/mule_preview/client/components.cljs#L119

Related Links:

FF Bug Reports:

@NoxHarmonium NoxHarmonium changed the title Call to requestAnimationFrame causes crash in Firefox Extensions Call to requestAnimationFrame causes crash in Firefox browser extensions Jul 30, 2019
@NoxHarmonium
Copy link
Author

Also I noticed that React also uses requestAnimationFrame to run its scheduler and it assigns it to a variable in a similar way. It might be worth checking if pure React suffers from the same issue. Does Reagent use the React scheduler at all?

@NoxHarmonium
Copy link
Author

Sample project to reproduce the issue is at https://github.com/NoxHarmonium/reagent-issue-438-sample

Next step is doing some more testing of my suggested fix, cleaning it up and submitting a PR

@Deraen
Copy link
Member

Deraen commented Apr 16, 2020

The proposed fix makes sense, but the example repository is missing dist/background.js referred by the manifest file so I can't test this.

@Deraen Deraen added the bug label Apr 16, 2020
@NoxHarmonium
Copy link
Author

Oops you're right, I had dist in .gitignore and it wasn't checked in. I just did a git clean and I've lost all the extension files. I'll let you know when its fixed up.

@NoxHarmonium
Copy link
Author

NoxHarmonium commented Apr 19, 2020

@Deraen turns out that the reference to background.js was just a copy and paste error. I've removed that.

I've also updated the dependencies to the latest versions and retested with the latest Chrome/Firefox on Fedora 31.

I noticed when testing it again that if I install the extension into my everyday Firefox profile, the issue still occurs but the logs are hidden for some reason. I think due to some conflict with another extension. Therefore I think the best way to show the issue on Firefox is to use web-ext by running:

$ npx web-ext run --source-dir . --start-url https://www.example.com

The extension will be installed in a clean temporary profile and its easy to see the issue in the browser console (not the web console).

I've updated the README to explain this.

I hope that helps!

@Deraen Deraen closed this as completed in 66ff6cc Apr 19, 2020
@Deraen
Copy link
Member

Deraen commented Apr 19, 2020

Thanks! Fix applied to master now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants