Support iframes (nested browsing contexts) in selection event handling (stale version) #7936

acusti · 2016-10-11T04:10:51Z

As mentioned in #7866 (comment), this PR takes the approach of checking all nested browsing contexts and saving and restoring their selections regardless of whether or not they are the primary focused element on the page. It is a more complete alternative to the solution in #7866, and like that PR, fixes #427. It is also, like the original, dependent on facebook/fbjs#188, but that PR has been merged into master, so as soon as a new fbjs version is released, this will be ready to go.

acusti · 2016-10-13T07:05:48Z

You can see this version of the solution in practice with an actual use case in this demo: http://codepen.io/acusti/pen/yajJbZ?editors=0010

The essential point is that I no longer need to manually manage focus in the app code and instead can rely on the ReactInputSelection functionality to take care of preserving visible “focus” (i.e. visible selections).

acusti · 2016-10-13T14:57:07Z

If anyone wants to play around with this, I pushed up a copy of the built react and react-dom scripts:

https://rawgit.com/Brandcast/react/nested-browsing-contexts-2-built/build/react.js
https://rawgit.com/Brandcast/react/nested-browsing-contexts-2-built/build/react-dom.js

acusti · 2016-10-15T14:53:44Z

src/renderers/dom/client/__tests__/ReactInputSelection-test.js

+
+    it('gets and restores selection for inputs in an iframe that get remounted', () => {
+      var iframe = document.createElement('iframe');
+      iframe.setAttribute('tabIndex', 0);


Setting tabIndex on the iframe element here and then invoking iframe.focus() below at line 202 was only necessary because jsdom didn’t handle setting activeElement in the parent correctly. This is fixed in jsdom v9.7.0, so if we updated jest’s jsdom dependency, I could remove the hacky workaround.

Updating the version of jsdom used by jest has landed (includes a fix of a major perf regression, so it’ll be fun to see how that affects test running times), so when a new version is released, should be able to remove the hacky workarounds in this test.

acusti · 2016-10-18T06:44:15Z

(EDIT: I updated the commit hash in the react dependency string below because the previous commit I pointed to didn’t have the correct lib/* files)

If anyone wants npm-installable versions of this PR to experiment with, you can use these in your package.json (requires the latest fbjs changes in master which have not yet been released, which is why that dependency also needs to be included):

    "fbjs": "Brandcast/fbjs-built#97b8e54",
    "react": "Brandcast/react#9a81d1a",

The dependency on fbjs changes also means the tests won’t pass until there’s a new release and that dep can be updated. Specifically, the assertions on the result of ReactInputSelection.getSelectionInformation() around this line and below.

acusti · 2016-11-10T16:55:37Z

@spicyj I’ve put together a Draft.js demo of this PR along with the Draft.js PR to illustrate the utility of making React work across nested browsing contexts. The PR is facebookarchive/draft-js#765, and the demo is here: http://codepen.io/acusti/pen/RGEJZE?editors=0011

In that demo, I used built versions of React and Draft.js which include all the changes for working across nested browsing contexts. Those built versions are usable in node-land or the browser:

EDIT See later comment for updated usable deps

In node

    "draft-js": "Brandcast/draft-js#f9affa3",
    "fbjs": "Brandcast/fbjs-built#97b8e54",
    "react": "Brandcast/react#9a81d1a",

In the browser

    <script src="https://rawgit.com/Brandcast/react/nested-browsing-contexts-2-built/build/react.js"></script>
    <script src="https://rawgit.com/Brandcast/react/nested-browsing-contexts-2-built/build/react-dom.js"></script>
    <script src="https://cdn.jsdelivr.net/immutable.js/latest/immutable.min.js"></script>
    <script src="https://cdn.jsdelivr.net/es6.shim/0.35.1/es6-shim.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/babel-core/5.8.34/browser.js"></script>
    <script src="https://rawgit.com/Brandcast/draft-js/iframe-compatibility-built/dist/Draft.js"></script>

I also rebased my branch (for this PR to React) to remove all upstream conflicts.

acusti · 2016-12-02T15:40:02Z

Just updated the PR after rebasing with master. There was a conflict in ReactInputSelection as a result of the new code added in #8401 from @spicyj to preserve scroll position when restoring selection and focusing an element. I incorporated that new functionality into these changes.

I also verified that I could remove the hacky workaround in the new ReactInputSelection tests that I had mentioned previously thanks to jsdom being updated to have more accurate iframe focus behavior, so the tests are a bit cleaner now. And npm test is all passing.

acusti · 2016-12-07T04:18:53Z

Thought I should mention that I’ve also updated my built versions of everything, including draft-js (v0.9.0), react (v15.4.1), and react-dom (v15.4.1). This is particularly important because the previous built versions were based on the React master branch 26 days ago (i.e. 16.0.0-alpha, or fiber), whereas these built versions are built from 15.4.1. To try out the PRs:

npm / yarn

    "draft-js": "brandcast/draft-js-built#333bf90",
    "fbjs": "brandcast/fbjs-built#3961252",
    "react": "brandcast/react-built#ab31c46",
    "react-dom": "brandcast/react-dom-built#6cd1db3",

in the browser

    <script src="https://rawgit.com/brandcast/react-built/15-stable/dist/react.js"></script>
    <script src="https://rawgit.com/brandcast/react-dom-built/15-stable/dist/react-dom.js"></script>
    <script src="https://cdn.jsdelivr.net/immutable.js/latest/immutable.min.js"></script>
    <script src="https://cdn.jsdelivr.net/es6.shim/0.35.1/es6-shim.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/babel-core/5.8.34/browser.js"></script>
    <script src="https://rawgit.com/brandcast/draft-js-built/master/dist/Draft.js"></script>

aleksey-shmatov · 2017-01-05T05:41:33Z

There is minor issue in Safari. I have one iframe with my content and one iframe for Google Maps. My website accessed with "http" and Google Maps hosted with "https". In the getElementsWithSelections following code does not throw error:
try {doc = win.document;} catch (e) {return acc;}
"doc" will be undefined after this code. This causes "undefined is not an object" exception and breaks functionality.

acusti · 2017-01-05T14:14:48Z

Interesting. I suppose just adding an additional early return if doc is falsy would handle that. Thanks for the feedback!

aleksey-shmatov · 2017-01-09T12:31:40Z

Thanks for adding this functionality, hope it get merged into master soon(together with draft-js). I have been working with this for about a week now and the only issue I encountered was that one for Safari.

philipp-spiess · 2017-02-09T15:06:46Z

@spicyj What's the status here? We'd need that behaviour but we can't afford to maintain a fork of React. Do you still have plans to merge this? Your last update is unfortunately already a few months old.

acusti · 2017-03-10T23:22:04Z

@sebmarkbage To clarify, did you remove the GH Review: review-needed label because someone did a review?

sophiebits · 2017-03-10T23:27:48Z

@acusti No, we have switched to using the github review feature for all PRs.

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti · 2017-03-15T13:49:57Z

I tried to rebase this branch from master and ran into a world of hurt, so I created a fresh branch off latest master cherry-picked over all the commits, and made a new PR: #9184

@aleksey-shmatov, I added a guard for the Safari bug you found in f8b202d, as well as dealing with a Firefox issue I found in my own testing. Thanks for trying it out and reporting back! I’ll hoping to make new built versions of everything off of these commits applied to 15-stable and update the npm / yarn instructions on using those soon. When I do, I will update both this now-stale PR and #9184 with the updated dependency strings.

Addresses facebook#7936 (comment) and summernote/summernote#1057

React generally handles being rendered into another window context correctly (we have been doing this for a while in native Mac popovers). The main place where there are global window/document accesses are in places where we deal with the DOM selection (window.getSelection() and document.activeElement). There has been some discussion about this in the public React GitHub repo: facebook/fbjs#188 facebook#7866 facebook#7936 facebook#9184 While this was a good starting point, those proposed changes did not go far enough, since they assumed that React was executing in the top-most window, and the focus was in a child frame (in the same origin). Thus for them it was possible to check document.activeElement in the top window, find which iframe had focus and then recurse into it. In our case, the controller and view frames are siblings, and the top window is in another origin, so we can't use that code path. The main reason why we can't get the current window/document is that ReactInputSelection runs as a transaction wrapper, which doesn't have access to components or DOM nodes (and may run across multiple nodes). To work around this I added a ReactLastActiveThing which keeps track of the last DOM node that we mounted a component into (for the initial render) or the last component that we updated (for re-renders). It's kind of gross, but I couldn't think of any better alternatives. All of the modifications are no-ops when not running inside a frame, so this should have no impact for non-elements uses. I did not update any of the IE8 selection API code paths, we don't support it.

Addresses facebook#7936 (comment) and summernote/summernote#1057

React generally handles being rendered into another window context correctly (we have been doing this for a while in native Mac popovers). The main place where there are global window/document accesses are in places where we deal with the DOM selection (window.getSelection() and document.activeElement). There has been some discussion about this in the public React GitHub repo: facebook/fbjs#188 facebook#7866 facebook#7936 facebook#9184 While this was a good starting point, those proposed changes did not go far enough, since they assumed that React was executing in the top-most window, and the focus was in a child frame (in the same origin). Thus for them it was possible to check document.activeElement in the top window, find which iframe had focus and then recurse into it. In our case, the controller and view frames are siblings, and the top window is in another origin, so we can't use that code path. The main reason why we can't get the current window/document is that ReactInputSelection runs as a transaction wrapper, which doesn't have access to components or DOM nodes (and may run across multiple nodes). To work around this I added a ReactLastActiveThing which keeps track of the last DOM node that we mounted a component into (for the initial render) or the last component that we updated (for re-renders). It's kind of gross, but I couldn't think of any better alternatives. All of the modifications are no-ops when not running inside a frame, so this should have no impact for non-elements uses. I did not update any of the IE8 selection API code paths, we don't support it. (cherry picked from commit 94b759b in the 0.14-stable. Appears to work mostly as is, needed to be updated to take 5c5d2ec into account)

acusti · 2017-10-31T14:33:34Z

For anyone who has been following along with this effort, I’m closing this in favor of a new PR: #11410

If you want to use the changes in the latest PR, which is based off of react 16, in your own project, you can do so by updating the react and react-dom dependencies in your package.json to:

    "react": "brandcast/react-built#7c8fc6b",
    "react-dom": "brandcast/react-dom-built#017534a",

If you’re also using draft.js and want the full latest and greatest cross-iframe experience, use:

    "draft-js": "brandcast/draft-js-built#003f24b",

Addresses facebook#7936 (comment) and summernote/summernote#1057

gaearon · 2018-08-02T22:24:09Z

We went with #12037

facebook-github-bot added GH Review: review-needed CLA Signed labels Oct 11, 2016

acusti mentioned this pull request Oct 11, 2016

Support iframes (nested browsing contexts) in selection event handling #7866

Closed

acusti force-pushed the nested-browsing-contexts-2 branch 6 times, most recently from 042f9dd to 489866b Compare October 13, 2016 05:54

facebook-github-bot added the CLA Signed label Oct 13, 2016

acusti force-pushed the nested-browsing-contexts-2 branch 2 times, most recently from 3b85ce4 to f6ee84e Compare October 13, 2016 06:19

facebook-github-bot added the CLA Signed label Oct 13, 2016

acusti force-pushed the nested-browsing-contexts-2 branch from ee466e5 to 403e4d6 Compare October 13, 2016 06:53

facebook-github-bot added the CLA Signed label Oct 13, 2016

acusti force-pushed the nested-browsing-contexts-2 branch from 6b03fb4 to 2a24dac Compare October 13, 2016 07:00

facebook-github-bot added the CLA Signed label Oct 13, 2016

acusti commented Oct 15, 2016

View reviewed changes

acusti force-pushed the nested-browsing-contexts-2 branch from 881a699 to 3dcc0d1 Compare October 15, 2016 14:59

facebook-github-bot added the CLA Signed label Oct 15, 2016

This was referenced Oct 18, 2016

Update jsdom dependency jestjs/jest#1944

Merged

Make DOM utilities work in nested browsing contexts facebook/fbjs#188

Merged

This was referenced Nov 5, 2016

Support rendering Draft.js into iframes (nested browsing contexts) facebookarchive/draft-js#765

Closed

DraftJS is not actually stateless facebookarchive/draft-js#527

Open

acusti force-pushed the nested-browsing-contexts-2 branch from 3dcc0d1 to 7592afd Compare November 10, 2016 02:51

acusti mentioned this pull request Nov 18, 2016

Attach event listeners at the root of the tree instead of document #8117

Closed

aweary mentioned this pull request Jan 26, 2017

Fix event handling when input name='nodeName' #6311

Closed

sebmarkbage removed the GH Review: review-needed label Mar 1, 2017

acusti force-pushed the nested-browsing-contexts-2 branch 2 times, most recently from 2ca7449 to b48f472 Compare March 15, 2017 04:43

acusti added a commit to brandcast/react that referenced this pull request Mar 15, 2017

Add guards for Firefox and Safari compatibility

f8b202d

Addresses facebook#7936 (comment) and summernote/summernote#1057

Add guards for Firefox and Safari compatibility

a97e39c

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti changed the title ~~Support iframes (nested browsing contexts) in selection event handling~~ Support iframes (nested browsing contexts) in selection event handling (stale version) Mar 15, 2017

acusti mentioned this pull request Mar 15, 2017

Support iframes (nested browsing contexts) in selection event handling #9184

Closed

acusti mentioned this pull request Mar 16, 2017

Make DOM utilities work in nested browsing contexts facebook/fbjs#222

Merged

acusti added a commit to brandcast/react that referenced this pull request Apr 9, 2017

Add guards for Firefox and Safari compatibility

3e24144

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti added a commit to brandcast/react that referenced this pull request May 4, 2017

Add guards for Firefox and Safari compatibility

d069d4f

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti added a commit to brandcast/react that referenced this pull request May 4, 2017

Add guards for Firefox and Safari compatibility

9e16980

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti added a commit to brandcast/react that referenced this pull request May 4, 2017

Add guards for Firefox and Safari compatibility

fb9107b

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti added a commit to brandcast/react that referenced this pull request Jul 27, 2017

Add guards for Firefox and Safari compatibility

4efe7f8

Addresses facebook#7936 (comment) and summernote/summernote#1057

acusti mentioned this pull request Oct 31, 2017

Support iframes (nested browsing contexts) in selection event handling for React 16 #11410

Closed

acusti closed this Oct 31, 2017

acusti added a commit to brandcast/react that referenced this pull request Nov 1, 2017

Add guards for Firefox and Safari compatibility

6ac703f

Addresses facebook#7936 (comment) and summernote/summernote#1057

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support iframes (nested browsing contexts) in selection event handling (stale version) #7936

Support iframes (nested browsing contexts) in selection event handling (stale version) #7936

acusti commented Oct 11, 2016

acusti commented Oct 13, 2016

acusti commented Oct 13, 2016

acusti Oct 15, 2016 •

edited

acusti Oct 18, 2016

acusti commented Oct 18, 2016 •

edited

acusti commented Nov 10, 2016 •

edited

acusti commented Dec 2, 2016 •

edited

acusti commented Dec 7, 2016

aleksey-shmatov commented Jan 5, 2017

acusti commented Jan 5, 2017

aleksey-shmatov commented Jan 9, 2017

philipp-spiess commented Feb 9, 2017

acusti commented Mar 10, 2017

sophiebits commented Mar 10, 2017

acusti commented Mar 15, 2017 •

edited

acusti commented Oct 31, 2017

gaearon commented Aug 2, 2018

Support iframes (nested browsing contexts) in selection event handling (stale version) #7936

Support iframes (nested browsing contexts) in selection event handling (stale version) #7936

Conversation

acusti commented Oct 11, 2016

acusti commented Oct 13, 2016

acusti commented Oct 13, 2016

acusti Oct 15, 2016 • edited

Choose a reason for hiding this comment

acusti Oct 18, 2016

Choose a reason for hiding this comment

acusti commented Oct 18, 2016 • edited

acusti commented Nov 10, 2016 • edited

EDIT See later comment for updated usable deps

acusti commented Dec 2, 2016 • edited

acusti commented Dec 7, 2016

npm / yarn

in the browser

aleksey-shmatov commented Jan 5, 2017

acusti commented Jan 5, 2017

aleksey-shmatov commented Jan 9, 2017

philipp-spiess commented Feb 9, 2017

acusti commented Mar 10, 2017

sophiebits commented Mar 10, 2017

acusti commented Mar 15, 2017 • edited

acusti commented Oct 31, 2017

gaearon commented Aug 2, 2018

acusti Oct 15, 2016 •

edited

acusti commented Oct 18, 2016 •

edited

acusti commented Nov 10, 2016 •

edited

acusti commented Dec 2, 2016 •

edited

acusti commented Mar 15, 2017 •

edited