Skip to content

Minimally support iframes (nested browsing contexts) in selection event handling #12037

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

Merged
gaearon merged 29 commits into from
Aug 2, 2018
Merged

Minimally support iframes (nested browsing contexts) in selection event handling #12037

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6940e33
Prefer node’s window and document over globals
acusti Jan 17, 2018
3f3c1d7
Support active elements in nested browsing contexts
acusti Jan 17, 2018
b08a270
Merge branch 'master' into minimal-iframes
acusti Jan 22, 2018
51be426
Avoid invoking defaultView getter unnecessarily
acusti Jan 23, 2018
3714067
Prefer node’s window and document over globals
acusti Jan 17, 2018
66b8766
Support active elements in nested browsing contexts
acusti Jan 17, 2018
75bc65e
Avoid invoking defaultView getter unnecessarily
acusti Jan 23, 2018
5a61b66
Implement selection event fixtures
Jan 29, 2018
f752792
Prefer node’s window and document over globals
acusti Jan 17, 2018
a8fcf05
Avoid invoking defaultView getter unnecessarily
acusti Jan 23, 2018
0003681
Fix react-scripts to work with alphas after 16.0.0
Feb 12, 2018
fef8519
Run prettier on new selection events fixtures
Feb 12, 2018
799ee39
Add fixture for onSelect in iframes, remove DraftJS fixture
Feb 13, 2018
6e3d4b7
Merge branch 'minimal-iframes' of github.com:brandcast/react into min…
acusti Mar 30, 2018
d1ad016
Purge remnants of draft.js from fixtures
acusti Apr 2, 2018
3c32963
Use prop-types import instead of window global
acusti Apr 2, 2018
3f7b5c5
Make fixtures’ Iframe component Firefox-compatible
acusti Apr 2, 2018
05d8969
Merge branch 'master' into minimal-iframes
acusti Apr 5, 2018
a776570
Merge branch 'master' into minimal-iframes
acusti May 17, 2018
26e9d82
Merge branch 'master' into minimal-iframes
wilsonhyng Jul 13, 2018
4db5c44
Fix switch case for SelectionEventsFixture
wilsonhyng Jul 16, 2018
ccf0329
Remove draft.js / immutable.js dependencies
wilsonhyng Jul 17, 2018
2edf1f8
Cache owner doc as var to avoid reading it twice
wilsonhyng Jul 17, 2018
db02b65
Add documentation for getActiveElementDeep to explain try/catch
wilsonhyng Jul 18, 2018
75b9992
Ensure getActiveElement always returns DOM element
wilsonhyng Jul 24, 2018
0f1de45
Tighten up isNode and isTextNode
wilsonhyng Jul 24, 2018
95b2aef
Remove ie8 compatibility
wilsonhyng Jul 24, 2018
d997ba8
Specify cross-origin example in getActiveElementDeep
wilsonhyng Aug 2, 2018
e63391c
Revert back to returning null if document is not defined
wilsonhyng Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Avoid invoking defaultView getter unnecessarily
  • Loading branch information
@acusti
acusti authored and Brandon Dail committed Feb 12, 2018
commit 75bc65eef9cd175e5999ea2e628fc52ce7e7e6dd
12 changes: 5 additions & 7 deletions packages/react-dom/src/client/ReactDOMSelection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ import {TEXT_NODE} from '../shared/HTMLNodeType';
* @return {?object}
*/
export function getOffsets(outerNode) {
let win = window;
if (outerNode.ownerDocument && outerNode.ownerDocument.defaultView) {
win = outerNode.ownerDocument.defaultView;
}
const win =
Copy link
Collaborator

@gaearon gaearon Jul 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: let's not read ownerDocument twice. Put it in a variable.

(outerNode.ownerDocument && outerNode.ownerDocument.defaultView) || window;
const selection = win.getSelection && win.getSelection();

if (!selection || selection.rangeCount === 0) {
Expand Down Expand Up @@ -155,12 +153,12 @@ export function getModernOffsetsFromPoints(
*/
export function setOffsets(node, offsets) {
const doc = node.ownerDocument || document;

if (!doc.defaultView.getSelection) {
const win = doc ? doc.defaultView : window;
if (!win.getSelection) {
Copy link
Contributor

@nhunzaker nhunzaker Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just doing some poking around:

image

In what cases does win not have getSelection, and would it be good to document this?

Copy link
Contributor Author

@acusti acusti Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of IE8, and possibly other very old browsers. We can remove the early return if that no longer matters.

Copy link
Contributor

@nhunzaker nhunzaker Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. That is fine by me. Sound good, @aweary?

return;
}

const selection = doc.defaultView.getSelection();
const selection = win.getSelection();
const length = node[getTextContentAccessor()].length;
let start = Math.min(offsets.start, length);
let end = offsets.end === undefined ? start : Math.min(offsets.end, length);
Expand Down
6 changes: 2 additions & 4 deletions packages/react-dom/src/events/SelectEventPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,8 @@ function getSelection(node) {
end: node.selectionEnd,
};
} else {
let win = window;
if (node.ownerDocument && node.ownerDocument.defaultView) {
win = node.ownerDocument.defaultView;
}
const win =
(node.ownerDocument && node.ownerDocument.defaultView) || window;
if (win.getSelection) {
Copy link
Contributor

@nhunzaker nhunzaker Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, when is getSelection absent?

Copy link
Contributor Author

@acusti acusti Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, in IE8

const selection = win.getSelection();
return {
Expand Down