Uh oh!
There was an error while loading. Please reload this page.
Using evaluateJavascript to fix #9749 - #9945
Conversation
ghost
commented
Sep 16, 2016
By analyzing the blame information on this pull request, we identified @sathyapriya-31 and @jvassbo to be potential reviewers. |
leeight
commented
Sep 19, 2016
Ping @mkonicek |
satya164
commented
Sep 19, 2016
Doesn't it mean that the behaviour becomes inconsistent across Android versions? People might test on new Android versions and not realize that the same code will break on older Android versions. |
leeight
commented
Sep 19, 2016
@satya164 So it's better tell the differences in the document, and display an YellowBox on older android devices? |
`evaluateJavascript` will preserve single line comment.
facebook-github-bot
commented
Oct 7, 2016
@leeight updated the pull request - view changes |
leeight
commented
Oct 7, 2016
facebook-github-bot
commented
Nov 6, 2016
It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @jacobp100 as a potential reviewer. Could you take a look please or cc someone with more context? |
jacobp100
commented
Nov 6, 2016
LGTM. There will obviously be some edge cases, where the ‘//‘ is in a string, but that probably is not worth trying to resolve. |
| if (__DEV__) { | ||
| if (this.props.injectedJavaScript | ||
| && this.props.injectedJavaScript.indexOf('//') !== -1) { |
There was a problem hiding this comment.
This will also cause warning if any string matches this
leeight
commented
Nov 7, 2016
OK, Close it. |
LinusU
commented
Jul 24, 2018
#20366 should fix this in a way that behaves the same on all versions of Android |
Thanks for submitting a pull request! Please provide enough information so that others can review your pull request:
Use
evaluateJavascriptinstead ofloadUrlif possible, which will preserve the single line comment.Test plan (required)
Manually.