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

Correctly handle different javascript realms/documents #4330

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

edave64
Copy link

@edave64 edave64 commented Jul 19, 2024

In JavaScript, multiple different global contexts (realms) can exist with their own classes and document (https://tc39.es/ecma262/#realm) and they can access each others objects.

These realms are created by calls to window.open or when using <iframe>s. An application I am working on enables multi-monitor support by opening multiple browser windows that are all controlled from the primary tab. And Quill does not function correctly when initialized in one of these external windows.

This can lead to very unintuitive behavior, since using the global document can lead to incorrect results and instanceof unexpectedly returning false. And because of https://bugzilla.mozilla.org/show_bug.cgi?id=1470017 in FireFox, we cannot even control what realm our initial objects are part of.

Luckily, nodes own references to their ownerDocument, which has a reference to their defaultView, making it possible to write code that survives these conditions.

This PR removes the expectation that document is the only document that can exist and fixes instanceof checks.

This requires slab/parchment#147 to work properly.

After this is merged, I will try to expand it to supporting shadowDOM as well, most of the basics are there, but it needs some bigger Emitter changes and don't really fit here. But this does already lay some groundwork and can clarify what tests will be needed for testing Quill in non-standard contexts.

@luin luin self-requested a review July 19, 2024 16:31
@luin luin self-assigned this Jul 19, 2024
@edave64 edave64 force-pushed the feature/multi-realm branch 2 times, most recently from 3f369cf to 91c211e Compare July 23, 2024 06:45
@edave64 edave64 force-pushed the feature/multi-realm branch from 91c211e to 903c913 Compare July 23, 2024 06:47
@edave64 edave64 marked this pull request as ready for review July 25, 2024 07:28
@edave64 edave64 changed the title WIP: Correctly handle different javascript realms/documents Correctly handle different javascript realms/documents Jul 25, 2024
@edave64
Copy link
Author

edave64 commented Jul 25, 2024

I've marked this as open, since it can already be reviewed and tested, but keep in mind that the E2E test will fail in Firefox until parchment is updated. (slab/parchment#147)

For local testing, the package.json can be modified to load parchment from a local checkout of that branch.

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.

2 participants