create3000 / x_ite

X_ITE X3D Browser, view and manipulate X3D and VRML scenes in HTML.
https://create3000.github.io/x_ite/
Other
66 stars 13 forks source link

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

Closed gwhitney closed 8 months ago

gwhitney commented 9 months ago

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 commented 9 months ago

Digging a bit further, according to https://github.com/marcj/css-element-queries/issues/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 commented 9 months ago

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.

create3000 commented 9 months ago

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 commented 9 months ago

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

create3000 commented 9 months ago

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

create3000 commented 9 months ago

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:

https://github.com/create3000/x_ite/blob/f2bf6bdd86a4ac37dd3207ed2d3d77d3da84368e/src/x_ite/Browser/Scripting/evaluate.js#L50

https://github.com/create3000/x_ite/blob/f2bf6bdd86a4ac37dd3207ed2d3d77d3da84368e/src/lib/nurbs/src/evaluator.js#L399

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

gwhitney commented 9 months ago

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 commented 9 months ago

New version 8.12.5 with latest changes is out now.

gwhitney commented 9 months ago

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