-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use ownerDocument instead of global document #2721
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,11 @@ const addUpUntilIndex = (sizes: number[], index: number): number => { | |
|
||
/* create a new style tag after lastEl */ | ||
const makeStyleTag = (target: ?HTMLElement, tagEl: ?Node, insertBefore: ?boolean) => { | ||
const el = document.createElement('style'); | ||
let targetDocument = document; | ||
if(target) targetDocument = target.ownerDocument; | ||
else if(tagEl) targetDocument = tagEl.ownerDocument; | ||
|
||
const el = targetDocument.createElement('style'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting that this causes no security issues; apparently any This can still break for CSP reasons, though, but it'd break regardless of whether we use CSSOM or child text nodes. |
||
el.setAttribute(SC_ATTR, ''); | ||
el.setAttribute(SC_VERSION_ATTR, __VERSION__); | ||
|
||
|
@@ -68,7 +72,7 @@ const makeStyleTag = (target: ?HTMLElement, tagEl: ?Node, insertBefore: ?boolean | |
} | ||
|
||
/* Work around insertRule quirk in EdgeHTML */ | ||
el.appendChild(document.createTextNode('')); | ||
el.appendChild(targetDocument.createTextNode('')); | ||
|
||
if (target && !tagEl) { | ||
/* Append to target when no previous element was passed */ | ||
|
@@ -226,7 +230,7 @@ const makeSpeedyTag = (el: HTMLStyleElement, getImportRuleTag: ?() => Tag<any>): | |
}; | ||
}; | ||
|
||
const makeTextNode = id => document.createTextNode(makeTextMarker(id)); | ||
const makeTextNode = (targetDocument, id) => targetDocument.createTextNode(makeTextMarker(id)); | ||
|
||
const makeBrowserTag = (el: HTMLStyleElement, getImportRuleTag: ?() => Tag<any>): Tag<Text> => { | ||
const names = (Object.create(null): Object); | ||
|
@@ -243,7 +247,7 @@ const makeBrowserTag = (el: HTMLStyleElement, getImportRuleTag: ?() => Tag<any>) | |
return prev; | ||
} | ||
|
||
markers[id] = makeTextNode(id); | ||
markers[id] = makeTextNode(el.ownerDocument, id); | ||
el.appendChild(markers[id]); | ||
names[id] = Object.create(null); | ||
|
||
|
@@ -281,7 +285,7 @@ const makeBrowserTag = (el: HTMLStyleElement, getImportRuleTag: ?() => Tag<any>) | |
if (marker === undefined) return; | ||
|
||
/* create new empty text node and replace the current one */ | ||
const newMarker = makeTextNode(id); | ||
const newMarker = makeTextNode(el.ownerDocument, id); | ||
el.replaceChild(newMarker, marker); | ||
markers[id] = newMarker; | ||
resetIdNames(names, id); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd write this as:
const targetDocument = target ? target.ownerDocument : tagEl ? tagEl.ownerDocument : document
but that's just my anti-mutability bias 🤷♀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jessidhia thank you for reviewing! I agree with anti-mutability, though, the eslint rule (no-nested-ternary) didn't let me do so :)