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 iframe crash #4219

Merged
merged 5 commits into from Apr 26, 2021
Merged

Conversation

juliankrispel
Copy link
Collaborator

@juliankrispel juliankrispel commented Apr 23, 2021

Description
Fixes iframe crashes

Context: The exception is thrown if an editor root element does not inherit from Document. However an document inside iframe will never inherit the same prototype as window.document, this change prevents triggering this exception if we are inside an iframe

Issue
Fixes: #4170

Example
Currently the iframe example crashes with the exception Unable to find DocumentOrShadowRoot for editor element ...
image

With this change it works again:
iframe-slate-fix

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)
  • You've added a changeset if changing functionality. (Add one with yarn changeset add.)

@changeset-bot
Copy link

changeset-bot bot commented Apr 23, 2021

🦋 Changeset detected

Latest commit: fdbcafc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@juliankrispel juliankrispel marked this pull request as draft April 23, 2021 08:02
@juliankrispel juliankrispel mentioned this pull request Apr 23, 2021
4 tasks
@juliankrispel
Copy link
Collaborator Author

🤔 One question that nags me is why do we throw an exception in findDocumentOrShadowRoot in the first place?
Wouldn't it be safer to remove the exception entirely or would this break something else and if so how can we test this.

Copying in @davidruisinger and @ianstormtaylor maybe you have some context I am missing

@juliankrispel juliankrispel marked this pull request as ready for review April 26, 2021 09:19
@juliankrispel
Copy link
Collaborator Author

juliankrispel commented Apr 26, 2021

  • Added smoke test for iframe example so that this doesn't regress

@juliankrispel juliankrispel merged commit 737aaa9 into main Apr 26, 2021
@juliankrispel juliankrispel deleted the 4170-fix-findDocumentOrShadowRoot-for-iframes branch April 26, 2021 13:47
@github-actions github-actions bot mentioned this pull request Apr 26, 2021
@clauderic
Copy link
Collaborator

clauderic commented Apr 26, 2021

I'm assuming this issue only happens when the editor is rendered within a portal DOM element that is in an iframe? Would have been good to clarify 🙏

@juliankrispel
Copy link
Collaborator Author

I'm assuming this issue only happens when the editor is rendered within a portal DOM element that is in an iframe? Would have been good to clarify 🙏

That is one of the issues yes (who knows what other usecases there are that fall into this category). In this case it's affecting the iframe example so https://www.slatejs.org/examples/iframe doesn't currently work (until this change is deployed).

@Nauss
Copy link

Nauss commented May 24, 2021

I was facing this issue in a different context.
I'm building an electron app where the user is able to create a report with rich texts (slate editors in a page). The user can then print his/her report. To do that I use an external child window created with window.open where I render the slate editors through a Portal.
This PR fixes the problem but while trying to fix this bug myself (before seeing this pull request), I tried the following:

var thisWindow = ReactEditor.getWindow(editor);
if (!(root instanceof thisWindow.Document || root instanceof thisWindow.ShadowRoot)) throw new Error("Unable to find DocumentOrShadowRoot for editor element: ".concat(el));

This works because root is still a Document but from the child window, not the parent.
Not sure if this would work with an iframe...

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.

slate-react 0.62.0 - Chromium iframe crash
3 participants