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

fix(): SCEM-5311 - Make Tidy3D plot_3d compatible with script limits #1586

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

tim-goddard-flexcompute
Copy link
Contributor

The Tidy3D notebooks environment is about to introduce a number of limits on script content in cell output as a security precaution. This conflicts with the way the plot_3d function operates, which is dependent on the ability to output JavaScript.

This change modifies the injected JavaScript to function whether the script is evaluated inline, or a similar copy is included in the environment. Our notebooks environment will add this script at the same time as introducing the change to strip script output from cells, preserving the ability to render 3d plots.

The script has been included inline, rather than from an external source, to most closely mimic the existing behavior, avoid adding an external script inclusion to customer environments.

@tim-goddard-flexcompute tim-goddard-flexcompute requested review from tylerflex and majinge7846 and removed request for tylerflex April 8, 2024 01:09
@tylerflex
Copy link
Collaborator

Thanks @tim-goddard-flexcompute, looks good to me. I'm just trying to figure out whether to release this as a patch 2.6.4 or whether it can wait a few weeks to release in our 2.7.0. Is it an urgent fix?

@tylerflex
Copy link
Collaborator

Also, just wondering whether this has been tested in eg. a notebook or if that's still needed?

@tylerflex
Copy link
Collaborator

FYI just tested on StartHere.ipynb and works as expected

image

@tylerflex tylerflex merged commit a39250a into develop Apr 10, 2024
15 checks passed
@tylerflex tylerflex deleted the tim/xss-prevention branch April 10, 2024 02:10
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

Successfully merging this pull request may close these issues.

3 participants