Skip to content

Error: Invalid target element for this operation -- IE9 #515

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

Jump to bottom
Closed
subtleGradient opened this issue Nov 11, 2013 · 11 comments
Closed

Error: Invalid target element for this operation -- IE9 #515

subtleGradient opened this issue Nov 11, 2013 · 11 comments
Labels
Type: Bug

Comments

@subtleGradient
Copy link
Contributor 

rendering React components at document should be able to get root component id for document node.

Error: Invalid target element for this operation.

rendering React components at document should be able to unmount component from document node.

Error: Invalid target element for this operation.

rendering React components at document should be able to switch root constructors via state.

Error: Invalid target element for this operation.

rendering React components at document should be able to switch root constructors.

Error: Invalid target element for this operation.

rendering React components at document should be able to mount into document.

Error: Invalid target element for this operation.
@subtleGradient
Copy link
Contributor Author 

mutateHTMLNodeWithMarkup should mutate the document html.

Error: Invalid target element for this operation.

mutateHTMLNodeWithMarkup should change attributes.

Error: Invalid target element for this operation.

@petehunt
Copy link
Contributor

I'm starting to think that full-page react isn't worth supporting. It's soooo complicated to get right and every browser is different :( Thoughts?

@andreypopp
Copy link
Contributor

@petehunt :(

Googling the issue it seems that document.documentElement.innerHTML is read-only in IE up to 9 (can't find anything more trustful than SO threads though). So maybe just wait for ie9 to retire or fix mutate mutateHTMLNodeWithMarkup by mutating <head> and <body> elements separately

@andreypopp
Copy link
Contributor

Ok, from MSDN

The innerHTML property is read-only on the col, colGroup, frameSet, html, head, style, table, tBody, tFoot, tHead, title, and tr objects.

So <head> reconciliation should be done my rearranging nodes. Which is by the way more desirable cause it prevents flashing due to stylesheet reappending — see facebookarchive/react-page#32

@petehunt
Copy link
Contributor

I guess we should just try to power through this and see how bad it is, eh?

@subtleGradient
Copy link
Contributor Author

#oldestBugEvar
This is how MooTools handles the innerHTML problem. It also effects Firefox < 4 in some cases.
https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L1021-L1049

@zpao
Copy link
Member

zpao commented Nov 11, 2013

We have something like that already - https://github.com/facebook/react/blob/master/src/vendor/core/getMarkupWrap.js

@andreypopp
Copy link
Contributor

@zpao it requires a bit more — for example it should take special care of <link> elements not to move them around if the new ones are same as the previous because that would trigger a new HTTP request for stylesheets, for example.

I have a patch which does that but it is way too hacky, for example mutateHTMLNodeWithMarkup.js now looks like this — https://gist.github.com/andreypopp/7474901, other changes are minimal. But the worst about this, I think, is that it introduces one more special case...

I use whole page components in react-app but now I have a branch without them. It's not so beautiful but more robust, I think.

Anyway, I'd be fine if we remove support for whole page components from React.

@petehunt
Copy link
Contributor

How about we remove support and throw if someone tries to do something dangerous?

This was referenced Nov 21, 2013
Closed
Closed
@sophiebits sophiebits mentioned this issue Dec 27, 2013
Closed
@petehunt
Copy link
Contributor

I just sent out some new diffs internally that restrict full-page rendering to only supporting a single root that never changes (ie no unmounting) and that it must be used with server rendering. This way we can avoid innerHTMLing scary things like documentElement while still supporting (albeit a bit "clunkily") most people's use cases for this.

@petehunt
Copy link
Contributor

petehunt commented Jan 6, 2014

https://groups.google.com/forum/#!topic/reactjs/4jI5xe7TXzQ

@petehunt petehunt closed this as completed Jan 6, 2014
@htor htor mentioned this issue Dec 11, 2018
Closed
@snyk-bot snyk-bot mentioned this issue Apr 2, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@subtleGradient @zpao @andreypopp @petehunt