Support rendering Draft-js into iframe #1877

haikyuu · 2018-09-21T15:49:05Z

Summary
This pull request adds support for draft-js usage inside iframes. There is another WIP PR which tries to achieve the same thing #765.

Note that this was mainly broken because instanceof does not check objects across iframes.
In other words (code) iframeElement instanceof Element === false while iframeElement instanceof iframeDocument.Element === true.

And the other reason is getSelection depends on the window object as well.

Checking if a node is instance of `Element`, `HTMLElement` ...

Depending on the context, we need a way to access the right iframe window object to check using instanceof.

Solutions	D	Pros 👍	Cons 👎	Shortcomings
attach the iframeWindow to global object, and use it to check whenever it's available	🛑	- Easy access to iframe document	- breaks when there is an instance of Draft-js inside the iframe and another outside.- breaks when there are +2 iframes as well
get the document from the node itself and check	✅	- Works fine even when there are many iframes or editors in and out iframes		- you can only access the global document when you have access to a node. (not needed here)
check the `nodeType` property instead (used in acusti's PR )	🛑		- can't differentiate between Element and HTMLElement

Challenges

When pasting something from a different context. We get a body element which document object doesn't have a corresponding window / defaultView . It was fixed by adding another check.

Accessing the window or document object in other cases `getSelection` ...

Whenever the event is accessible, we use it to access the ownerDocument

Places skipped (they use the document and global objects)

getSafeBodyFromHTML : it creates a new document anyway
getTextContentFromFiles : it uses the global.fileReader

Other challenges

In order to fix some flow errors, i had to force casting node types in some places. Note that i only did this after instanceof checks, so it's safe. Those checks were not detected by flow because we use instanceof to check in iframe window or in the main window. One solution to fix this all together is to check nodes using a utility like fbjs' isNode but i prefer to keep the changes minimal.

Known issues

Inside an editor in an iframe: selecting some text and hitting cmd+b would apply the inline style but the selection will collapse. It happens only once, and doesn't happen outside an iframe. You can check it here and reproduce it in the sandbox

Test Plan

I created a sandbox to test draft-js editor. It has 2 editors inside separate iframes, and one in the main window.

We do that to access them from whenever it's needed.

claudiopro

Thanks for the PR @haikyuu! I'll run some internal smoke tests to ensure we're not regressing with these changes. Otherwise looking quite good. Please take a look at my comments in the meantime, especially the issue with getAnonymizedEditorDOM.

claudiopro · 2018-09-24T13:00:42Z

src/component/selection/setDraftEditorSelection.js

-    return document.createTextNode(
+    const documentObject = getCorrectDocumentFromNode(node);
+
+    return documentObject.createTextNode(


Can you chain the call to getCorrectDocumentFromNode(node) without an intermediate assignment here and in function addPointToSelection, since you're not reusing the value?

claudiopro · 2018-09-24T13:02:53Z

src/component/selection/setDraftEditorSelection.js

@@ -76,11 +81,10 @@ function getAnonymizedEditorDOM(
 ): string {
  // grabbing the DOM content of the Draft editor
  let currentNode = node;
+  // this should only be used after checking with isElement
+  let castedNode: Element = (currentNode: any);


You should also assign it on line 91 when reassigning currentNode = currentNode.parentNode

claudiopro · 2018-09-24T13:07:32Z

src/model/encoding/convertFromHTMLToContentBlocks2.js

-  );
+  if (isHTMLAnchorElement(node)) {
+    const castedNode: HTMLAnchorElement = (node: any);
+    return (


You lost a !! before the returned expression. I'd also suggest to return Boolean(<expression>) instead.

claudiopro · 2018-09-24T13:12:58Z

src/component/utils/isElement.js

@@ -0,0 +1,14 @@
+function isElement(node) {


You could also check if node.nodeType === Node.ELEMENT_NODE

claudiopro · 2018-09-24T13:17:18Z

src/component/utils/isHTMLAnchorElement.js

@@ -0,0 +1,14 @@
+function isHTMLAnchorElement(node) {


I'd implement this checking that, as @sophiebits suggested in another PR, isElement(node) && node.nodeName === 'A'

claudiopro · 2018-09-24T13:18:30Z

src/component/utils/isHTMLImageElement.js

+  if (!node || !node.ownerDocument) {
+    return false;
+  }
+  if (!node.ownerDocument.defaultView) {


isElement(node) && node.nodeName === 'IMG'

claudiopro · 2018-09-24T14:02:43Z

I also found an issue when the iframed Editor has a placeholder prop, the placeholder text is rendered as a text block and the caret is placed on the next line.

Repro: https://codesandbox.io/s/nk90w2jv9j

claudiopro · 2018-09-24T14:21:24Z

Ah, nevermind - that issue is caused by the absence of the Draft.css stylesheet 😄 apparently the placeholder is rendered using CSS.

claudiopro · 2018-09-24T14:25:39Z

@haikyuu I PR'd an example based on your CodeSandbox demo: #1879

claudiopro · 2018-10-21T18:34:27Z

@haikyuu do you need any help to address the review feedback? I'd like to merge this since it's in a good shape and there are many interested consumers!

haikyuu · 2018-10-21T20:19:53Z

I was quite busy these days. I'll fix them next week once i have some spare time👍

claudiopro · 2018-11-14T15:17:41Z

Hi @haikyuu, would you have time to look into the finishing touch to this PR?

Several Draft.js users who are eager to embed editors in an iframe would be thankful for your efforts 😄

marek-baranowski · 2018-11-17T18:36:56Z

Hey @haikyuu, I'm also blocked by the iframe issue and I would love to see this PR merged :)

haikyuu · 2018-11-19T15:36:24Z

@marek-baranowski @claudiopro continued the work on #1938 👍

niveditc · 2019-01-19T21:32:34Z

Closing this since it's continued on #1938 instead.

Summary: **Summary** Takes over and applies requested PR feedback to #1877 by haikyuu. Refer to original PR for context and discussion. **Test Plan** Run some manual smoke tests on the editor running in an iframe as follows: ``` yarn python3 -m http.server 8080 . open http://localhost:8080/examples/draft-0-10-0/iframe/iframe.html ``` Pull Request resolved: #1938 Reviewed By: claudiopro Differential Revision: D13137413 Pulled By: jack-arms fbshipit-source-id: efcdbabc7d8d2aff4fbebc8b06c22d57756ebc12

…ve#1938) Summary: **Summary** Takes over and applies requested PR feedback to facebookarchive#1877 by haikyuu. Refer to original PR for context and discussion. **Test Plan** Run some manual smoke tests on the editor running in an iframe as follows: ``` yarn python3 -m http.server 8080 . open http://localhost:8080/examples/draft-0-10-0/iframe/iframe.html ``` Pull Request resolved: facebookarchive#1938 Reviewed By: claudiopro Differential Revision: D13137413 Pulled By: jack-arms fbshipit-source-id: efcdbabc7d8d2aff4fbebc8b06c22d57756ebc12

haikyuu added 10 commits September 17, 2018 23:17

Attach provided iframeWindow and iframeDocument to global document.

60267ef

We do that to access them from whenever it's needed.

fix invariant crash when selecting. test instanceof in the iframe window

9dd86b5

fix verifying that nodes are instances of Element/HTMLElement

1fa0973

change global.getSelection to currentWindow.getSelection

acd6ad0

use correct document and window/global objects

cdaa02e

add checks for other elements

4dedfb3

fix flow errors

3534359

fix bug in getting defaultView from editor ref

c12d93f

Delete package-lock.json

a9979d0

fix getting defaultView from this.editor

d27b28b

facebook-github-bot added the CLA Signed label Sep 21, 2018

claudiopro suggested changes Sep 24, 2018

View reviewed changes

haikyuu mentioned this pull request Oct 16, 2018

Adds iframed editor example #1879

Closed

claudiopro pushed a commit to claudiopro/draft-js that referenced this pull request Nov 19, 2018

Commandeers and applies PR feedback to facebookarchive#1877

de81349

claudiopro mentioned this pull request Nov 19, 2018

Take over enhancements to render Draft.js in an iframe #1938

Closed

niveditc closed this Jan 19, 2019

mrkev mentioned this pull request Nov 20, 2019

Support rendering Draft.js into iframes (nested browsing contexts) #765

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support rendering Draft-js into iframe #1877

Support rendering Draft-js into iframe #1877

haikyuu commented Sep 21, 2018 •

edited

Loading

claudiopro left a comment

claudiopro Sep 24, 2018

claudiopro Sep 24, 2018

claudiopro Sep 24, 2018

claudiopro Sep 24, 2018

claudiopro Sep 24, 2018

claudiopro Sep 24, 2018

claudiopro commented Sep 24, 2018

claudiopro commented Sep 24, 2018

claudiopro commented Sep 24, 2018

claudiopro commented Oct 21, 2018

haikyuu commented Oct 21, 2018

claudiopro commented Nov 14, 2018

marek-baranowski commented Nov 17, 2018

haikyuu commented Nov 19, 2018

niveditc commented Jan 19, 2019

Support rendering Draft-js into iframe #1877

Support rendering Draft-js into iframe #1877

Conversation

haikyuu commented Sep 21, 2018 • edited Loading

Checking if a node is instance of Element, HTMLElement ...

Challenges

Accessing the window or document object in other cases getSelection ...

Places skipped (they use the document and global objects)

Other challenges

Known issues

claudiopro left a comment

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro Sep 24, 2018

Choose a reason for hiding this comment

claudiopro commented Sep 24, 2018

claudiopro commented Sep 24, 2018

claudiopro commented Sep 24, 2018

claudiopro commented Oct 21, 2018

haikyuu commented Oct 21, 2018

claudiopro commented Nov 14, 2018

marek-baranowski commented Nov 17, 2018

haikyuu commented Nov 19, 2018

niveditc commented Jan 19, 2019

haikyuu commented Sep 21, 2018 •

edited

Loading

Checking if a node is instance of `Element`, `HTMLElement` ...

Accessing the window or document object in other cases `getSelection` ...