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

Viewer fails to initialize inside cross-origin iframe #524

Open
themoenen opened this issue Feb 27, 2024 · 2 comments
Open

Viewer fails to initialize inside cross-origin iframe #524

themoenen opened this issue Feb 27, 2024 · 2 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@themoenen
Copy link

themoenen commented Feb 27, 2024

Describe the bug
The viewer can't be initialized when rendered inside a cross-origin iframe.
This is due to a window.top event listener and is easy to fix.

To Reproduce
Steps to reproduce the behavior:

  1. Create a folder miew-test

  2. Set up two web servers pointing to the same folder, to simulate a cross origin iframe.

    In one terminal run:

     python -m http.server 8000
    

    In another terminal run:

     python -m http.server 8001
    

    Note: Python 3 assumed. For Mac use python3

  3. Create viewer.html with a functioning viewer:

<!DOCTYPE html>
<html lang="en">
    <head>
        <link rel="stylesheet" href="https://unpkg.com/miew@0.9.0/dist/Miew.min.css" />
        <script src="https://unpkg.com/@babel/polyfill@7/dist/polyfill.min.js"></script>
        <script src="https://unpkg.com/lodash@4.17.15/lodash.js"></script>
        <script src="https://unpkg.com/three@0.112.1/build/three.min.js"></script>
        <script src="https://unpkg.com/miew@0.9.0/dist/Miew.min.js"></script>
    </head>
    <body>
        <h1>Miew Viewer</h1>
        <div class="miew-container" style="width: 640px; height: 480px"></div>

        <script>
            ;(function () {
                var viewer = new Miew({ load: '1CRN' })
                if (viewer.init()) {
                    viewer.run()
                }
            })()
        </script>
    </body>
</html>
  1. Create index.html with an iframe pointing at the viewer:
<!DOCTYPE html>
<html lang="en">
    <head>
    </head>
    <body>
        <iframe src="http://localhost:8001/viewer.html" width="800" height="600"></iframe>
    </body>
</html>
  1. Open http://localhost:8000

Expected behavior
The viewer should initialize successfully.

Actual behavior
The viewer doesn't initialize successfully.

Source of the problem
The two lines below are responsible. Because we're referring to window.top, this causes a CORS error blocking the further initialization of the viewer. Replacing window.top with window fixes the problem. If it's important to keep window.top, logic should be added to prevent the initialization from crapping out completely.

Miew.js

window.top.addEventListener('keydown', (event) => {
  self._onKeyDown(event);
});

window.top.addEventListener('keyup', (event) => {
  self._onKeyUp(event);
});

If window.top can be replaced, I presume it should also be updated in ObjectControls.js:

ObjectControls.prototype.getKeyBindObject = function () {
  return window.top;
};

Screenshots
Screenshot 2024-02-27 at 11 47 26 AM
Screenshot 2024-02-27 at 11 47 32 AM
Screenshot 2024-02-27 at 11 48 25 AM

Context

  • OS: MacOS Sonoma 14.3.1 (23D60)
  • Browser: Google Chrome
  • Version: Version 118.0.5993.117 (Official Build) (arm64)
  • npm Miew version: 0.10.1

Additional context
If you're ok with replacing window.top with window I can submit a pull request.

@themoenen themoenen added the bug Indicates an unexpected problem or unintended behavior label Feb 27, 2024
@paulsmirnov
Copy link
Member

Thank you for reporting the issue. We will investigate it and look for a solution to the problem. Unfortunately, replacing "window.top" with "window" would disable the behavior that we intended to implement originally. We might be ok with deprecating the original intents now but we must carefully consider all consequences.

Which miew version do you use? I'm asking because we don't have plans to make a new release soon. It would be great if you could patch your version to not be blocked by our release schedule. We are kind of limited in resources.

@themoenen
Copy link
Author

Thanks for the quick reply. I'm on the latest npm version (0.10.1)

I'm already hosting my own modified version, so there's no immediate rush to get this fixed.

I will look into wrapping this into a try/catch as to preserve binding events to the top window and submit a pull request if successfull.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants