Skip to content
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

rendering strings with newlines is inconsistent on re-rerender #1080

Closed
jbonta opened this issue Feb 13, 2014 · 9 comments · Fixed by #1081
Closed

rendering strings with newlines is inconsistent on re-rerender #1080

jbonta opened this issue Feb 13, 2014 · 9 comments · Fixed by #1081

Comments

@jbonta
Copy link
Contributor

jbonta commented Feb 13, 2014

On re-render, we sometimes use innerText. This causes inconsistencies in rendering whitespace. See demo:

http://jsfiddle.net/nbA2M/

We should make sure this doesn't happen.

@sophiebits
Copy link
Collaborator

We switched to use textContent but it broke some contentEditable stuff internal to FB. @zpao knows more.

@jbonta
Copy link
Contributor Author

jbonta commented Feb 13, 2014

I'm switching it back (D1174395), but regardless, this is an innerText issue. Browsers that don't have textcontent (ie8) suffer

@jbonta
Copy link
Contributor Author

jbonta commented Feb 13, 2014

looks like this was brought up in #923

@sophiebits
Copy link
Collaborator

Right, we should also do the right thing in IE8.

@sophiebits
Copy link
Collaborator

jQuery empties then adds a text node. Relevant commits:

jquery/jquery@866187f
jquery/jquery@74a132d (http://bugs.jquery.com/ticket/1264)

@jbonta
Copy link
Contributor Author

jbonta commented Feb 14, 2014

okay interesting. I'll do this for ie8

@sophiebits
Copy link
Collaborator

@jbonta I have a patch almost ready.

zpao pushed a commit that referenced this issue Feb 14, 2014
reverts 23ab30f because the issue it fixed doesn't seem to be a problem
anymore. textContent should be better for more things anyway, as the
original 309a88b indicates. It should be faster because innerText
requires layout.

This also allows us to use strings with newlines in our rendered output
and have confidence that they will render the same on subsequent
re-renders. This is especially useful for es6-style backtick multi-line
strings. This will be more consistent as textContent is supported almost
everwhere so we won't have differing behavior between webkit and
firefox.

see #1080
sophiebits added a commit to sophiebits/react that referenced this issue Feb 14, 2014
@syranide
Copy link
Contributor

@spicyj I'm curious if https://github.com/facebook/react/blob/master/src/browser/ReactDOMIDOperations.js#L144 is a simpler solution to the newline problem (for IE8), or if that only works for innerHTML.

@jbonta
Copy link
Contributor Author

jbonta commented Feb 14, 2014

hmm interesting. We definitely want to preserve whitespace though. (because of <pre > and white-space:pre and <textarea>)

jgebhardt pushed a commit to jgebhardt/react that referenced this issue Feb 15, 2014
reverts 23ab30f because the issue it fixed doesn't seem to be a problem
anymore. textContent should be better for more things anyway, as the
original 309a88b indicates. It should be faster because innerText
requires layout.

This also allows us to use strings with newlines in our rendered output
and have confidence that they will render the same on subsequent
re-renders. This is especially useful for es6-style backtick multi-line
strings. This will be more consistent as textContent is supported almost
everwhere so we won't have differing behavior between webkit and
firefox.

see facebook#1080
toptaldev92 pushed a commit to toptaldev92/react_project that referenced this issue Jul 28, 2021
reverts 23ab30ff873fba816129cafa0c40112dd2ee990d because the issue it fixed doesn't seem to be a problem
anymore. textContent should be better for more things anyway, as the
original 309a88bcf62c1c64a2b91c0eb753c19209a425b2 indicates. It should be faster because innerText
requires layout.

This also allows us to use strings with newlines in our rendered output
and have confidence that they will render the same on subsequent
re-renders. This is especially useful for es6-style backtick multi-line
strings. This will be more consistent as textContent is supported almost
everwhere so we won't have differing behavior between webkit and
firefox.

see facebook/react#1080
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants