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

Can't use x_ite from inside a Firefox extension; can ResizeSensor.js be pruned? #155

Closed
gwhitney opened this issue Sep 28, 2023 · 9 comments
Labels
enhancement New feature or request fixed The issue is already fixed

Comments

@gwhitney
Copy link
Contributor

If one creates a Firefox extension and attempts to load x_ite from a content script in the extension, it fails with an error that
Function('return this')() violates the Content Security Policy. And indeed, no direct or indirect evaluation of strings is allowed in a Firefox extension. This is blocking my efforts to create a browser extension that would display certain sorts of 3D content inline automatically in certain types of web pages.

But the offending code is not directly part of x_ite. Rather, it is inherited from the ResizeSensor.js file from the css-element-queries package bundled into x_ite.js/.mjs. (It is part of some slightly esoteric code that seeks to obtain the "true" global window object to access the "real" requestAnimationFrame function. To allow in general what ResizeSensor.js is trying to do, Firefox has relatively recently added a new global identifier globalThis which could be subbed in to ResizeSensor.js to provide the necessary functionality in a way consistent with the Content Security Policy.)

But the simplest fix to the problem at hand with x_ite would be to prune ResizeSensor.js. The only place that I can find that it is mentioned is in src/x_ite.html. What purpose is it serving there/is it still actually necessary? Could its use possibly be eliminated altogether?

Even if not, although I am not yet really widely familiar with the x_ite code base, it seems at first glance that src/x_ite.html is only used for testing? If that's right, then could ResizeSensor be removed from the x_ite.js/.mjs bundle, and only loaded separately into x_ite.html? (The fact that it is only listed in devDependencies in package.json seems to support that it needn't necessarily be part of the delivered bundle.) Removing it would reduce the size of the bundle a bit, loosen the dependencies, and forestall this unfortunate side effect of css-element-queries on Firefox extensions. (I have verified that if one patches the underlying code in ResizeSensor.js, then x_ite can be loaded in an extension, so it is the only blocker.)

If you think it could be pruned from the project entirely or at least from the bundle, please just give me a word or two of guidance as to how to proceed with that and I will be happy to prepare a PR against the development branch. The only other alternative to get back on track with using x_ite the way I have hoped all along would be to submit a change to the css-element-queries package and then if it is accepted, update here to that putative new version of css-element-queries. I will go ahead and prepare a PR for that project, but I am a bit worried in that the last merge to it occurred over three years ago. And whether or not css-element-queries is fixed, if it's indeed possible to prune it from x_ite (project or bundle) that would ultimately be better for x_ite.

Thanks for your thoughts/guidance.

@gwhitney
Copy link
Contributor Author

Digging a bit further, according to marcj/css-element-queries#309 one can replace ResizeSensor altogether with a very lightweight wrapper around ResizeObserver, which is built in to all modern browsers for quite some time. Since css-element-queries has a few PRs that have been open for 2-3 years, I don't expect to get it to change. Hence, with your consent and guidance, I'd like to remove it from x_ite one way or another. Please let me know whether you think it is still used/needed, in which case I will try to replace it with ResizeObserver, or whether I should simply try to prune ResizeSensor altogether. Thanks.

@gwhitney
Copy link
Contributor Author

P.S. Of course I could patch the x_ite bundle, and that is what I am doing for testing, but as you may or may not know, the Mozilla organization will refuse to sign extensions that patch any third-party libraries they use; all third-party libraries must be delivered exactly as released by the author. And without Mozilla's signature, extensions are not allowed to be installed in anyone's Firefox browser. That's why for my project to move forward, I really need to help produce an official version of the x_ite bundle that can be loaded in an extension.

gwhitney added a commit to gwhitney/x_ite that referenced this issue Sep 28, 2023
  In case it is of any help with create3000#155, this PR represents starting with
  the current development branch, performing `npm remove css-element-queries`,
  deleting all occurrences of ResizeSensor and css-element-queries (they
  only occurred in src/x_ite.html and webpack.config.js anyway), correcting
  the casing of `GifMedia.js` in
  `src/x_ite/Components/Texturing/MovieTexture.js` (so that webpack will
  execute on Linux, which is case-sensitive), and then running `make dist`.
  The exact result of all that is committed in this PR.

  I don't actually know that this is a reasonable change, but I thought
  I would provide it for your convenience. In particular, I couldn't find
  any sort of developer's guide among the documentation, so I wasn't sure
  what all to do to try out/test a change like this. I did try "make tests"
  but it did not run at all (even before I made any changes to the source).

  Hope this helps in some way; looking forward to getting a version of
  x_ite that can be loaded in a browser extension.
@create3000
Copy link
Owner

That's the first time I hear from ResizeObserver, but indeed we should use it now. :-)

ResizeSensor is used in src/x_ite/Browser/Rendering/X3DRenderingContext.js, which should then be replaced by ResizeObserver.

@gwhitney
Copy link
Contributor Author

Ah, not sure how I missed it in there. I will replace it with ResizeObserver and update my PR.

@create3000
Copy link
Owner

Merged your PR yesterday and with a small adjustment it works very well. Thank you very much for your contribution.

@create3000 create3000 added the enhancement New feature or request label Sep 29, 2023
@create3000
Copy link
Owner

I don't know if it will become a problem for your project, but in the X_ITE source code there are some new Function ("...") statements:

return Function (/* js */ `with (arguments [0])

var evaluator = new Function([code.join("\n"), "; return ", functionName].join(""))();

These code lines are essential for Script node and NURBS rendering.

@gwhitney
Copy link
Contributor Author

Thanks for fixing up my PR to remove ResizeSensor, it's very much appreciated and I look forward to the release.

As to your question about whether the Scripting and nurbs uses of new Function("....") will cause problems for my project: I do think it means that if inside my extension I try to view x3d files that use Script nodes or NURBS, they will fail to execute. However, I have already verified (with my locally patched version of x_ite.mjs) that at least the large majority of files I am trying to display do not use these parts of X3D. And fortunately, the ban on Function("...code...") is not against such an expression occurring, but rather against such an expression being executed. Eventually in my extension I will need to catch the errors that occur when it tries to execute a new Function("...") and fail gracefully (maybe report to the user they are trying to view a file with script/nurbs and those are not supported), but that will be a much later polishing step since basically it is working now for the files I care about. Thanks!

@create3000 create3000 added the fixed The issue is already fixed label Sep 30, 2023
@create3000
Copy link
Owner

New version 8.12.5 with latest changes is out now.

@gwhitney
Copy link
Contributor Author

gwhitney commented Oct 6, 2023

And working nicely for me in a Firefox extension, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed The issue is already fixed
Projects
None yet
Development

No branches or pull requests

2 participants