Skip to content

injected javascript comments now work on android#12321

Closed
mishuagopian wants to merge 1 commit intofacebook:masterfrom
mishuagopian:comment-on-android-webview-fix
Closed

injected javascript comments now work on android#12321
mishuagopian wants to merge 1 commit intofacebook:masterfrom
mishuagopian:comment-on-android-webview-fix

Conversation

@mishuagopian
Copy link

@mishuagopian mishuagopian commented Feb 10, 2017

Summary

When injecting a script to a WebView component via injectedJavaScript prop, if the script contains any comment then it won't work on android (issue: #9749).

Test plan

Create a WebView in which we inject a script with a simple comment in it.

Code to reproduce

import React from 'react';
import { AppRegistry, StyleSheet, Text, View, WebView } from 'react-native';

function SampleApp () {
  const html = `
    <html>
      <body>
        <h1 class="title"> Hello! </h1>
      </body>
    </html>
  `;
  const script = `
    var text = "Bye!"; // This comment will break the script on android
    document.querySelector(".title").textContent = text;
  `;
  return (
    <View style={styles.container}>
      <Text> WebView script comment issue </Text>
      <WebView
        source={{html: html}}
        injectedJavaScript={script}
        javaScriptEnabled
      />  
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'stretch',
    backgroundColor: '#F5FCFF',
    paddingTop: 50
  }
});

AppRegistry.registerComponent('SampleApp', () => SampleApp);

Expected Results

Text on html h1 element ("Hello!") should be replaced with "Bye!" text.

Actual Results

Text on h1 html element changes on iOS but not on Android. Removing the comment on the js script will fix this issue.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Feb 10, 2017
@mishuagopian mishuagopian mentioned this pull request Feb 10, 2017
Closed
@janicduplessis
Copy link
Contributor

janicduplessis commented Feb 10, 2017

Does this means it will only work properly with comments on kitkat+?

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 4, 2017

@mishuagopian I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

shergin
shergin suggested changes Aug 29, 2017
View reviewed changes
ReactAndroid/src/main/java/com/facebook/react/views/webview/ReactWebViewManager.java
injectedJS != null &&
!TextUtils.isEmpty(injectedJS)) {
loadUrl("javascript:(function() {\n" + injectedJS + ";\n})();");
String code = "(function() {\n" + injectedJS + ";\n})();";
Copy link
Contributor

@shergin shergin Aug 29, 2017

Choose a reason for hiding this comment

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

Why do we need this wrapping here?

@react-native-bot react-native-bot added Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. Ran Commands One of our bots successfully processed a command. Android labels Mar 16, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
This was referenced Jun 7, 2018
Closed
Closed
@LinusU LinusU mentioned this pull request Jul 24, 2018
Closed
kelset pushed a commit that referenced this pull request Aug 13, 2018
@LinusU @kelset
Properly escape JavaScript code on Android (#20366)
e4f6694 
Summary:
These changes will fix executing javascript with any special characters, by making use of the `evaluateJavascript` function on Android 4.4+, and by properly escaping the URI on Android <4.4.

Fixes #19611 • Fixes #20365 • Fixes #9749 • Closes #19655 • Closes #12321

This PR supersedes #19655 by patching the same problem in all the places, and fixing it for Android <4.4 as well.
Pull Request resolved: #20366

Differential Revision: D9242968

Pulled By: hramos

fbshipit-source-id: f2e1abc786ba333dbd8aaa8922e716fd99ec26e0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@shergin shergin shergin requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. Platform: Android Android applications. Ran Commands One of our bots successfully processed a command.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@mishuagopian @janicduplessis @facebook-github-bot @shergin @lacker @hramos @react-native-bot