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

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
Specify cross-origin example in getActiveElementDeep
  • Loading branch information
@wilsonhyng
wilsonhyng committed Aug 2, 2018
commit d997ba8ee18d39c308d65f07b080ec45bae8ba27
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactInputSelection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function getActiveElementDeep() {
let element = getActiveElement();
while (element instanceof win.HTMLIFrameElement) {
// Accessing the contentDocument of a HTMLIframeElement can cause the browser
// to throw, e.g. if the src is not in the same domains the parent document
// to throw, e.g. if it has a cross-origin src attribute
try {
win = element.contentDocument.defaultView;
} catch (e) {
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.

What exactly are we catching here? Is it defaultView access throwing? Or do we expect contentDocument to potentially be missing?

Copy link
Contributor Author

@acusti acusti Jul 17, 2018

Choose a reason for hiding this comment

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

If you just try to access the contentDocument of an HTMLIframeElement that comes from a non-matching (separate) domain (like if there’s an embedded YouTube player in the DOM), the browser will throw an error.

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.

Could you add a comment to this line describing such a scenario?

Expand Down