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.

Sign up for GitHub

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

Jump to bottom

Viewer fails to initialize inside cross-origin iframe #524

Closed
themoenen opened this issue Feb 27, 2024 · 3 comments · Fixed by #550
Closed

Viewer fails to initialize inside cross-origin iframe #524

themoenen opened this issue Feb 27, 2024 · 3 comments · Fixed by #550
Assignees
@paulsmirnov
Labels
bug Indicates an unexpected problem or unintended behavior
Milestone
v0.11.0 "Revival"

Comments

@themoenen
Copy link

themoenen commented Feb 27, 2024 •
edited
Loading

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.

@themoenen themoenen mentioned this issue Mar 27, 2024
Closed
5 tasks
@paulsmirnov paulsmirnov mentioned this issue Mar 29, 2024
Closed
@paulsmirnov paulsmirnov self-assigned this Aug 12, 2024
@paulsmirnov paulsmirnov added this to the v0.11.0 "Revival" milestone Aug 12, 2024
@paulsmirnov paulsmirnov mentioned this issue Aug 20, 2024
Merged
5 tasks
@paulsmirnov
Copy link
Member

Hopefully, fixed in #550

shot-20240819-212136

shot-20240819-212716

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paulsmirnov paulsmirnov

Labels
bug Indicates an unexpected problem or unintended behavior
Projects
Miew
Archived in project
Milestone
v0.11.0 "Revival"
Development

Successfully merging a pull request may close this issue.

Access the top window in a safe manner epam/miew
2 participants
@themoenen @paulsmirnov