Skip to content

Conversation

@mishuagopian
Copy link

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
@janicduplessis
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@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.

injectedJS != null &&
!TextUtils.isEmpty(injectedJS)) {
loadUrl("javascript:(function() {\n" + injectedJS + ";\n})();");
String code = "(function() {\n" + injectedJS + ";\n})();";
Copy link
Contributor

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
kelset pushed a commit that referenced this pull request Aug 13, 2018
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 #19611Fixes #20365Fixes #9749Closes #19655Closes #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

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

Development

Successfully merging this pull request may close these issues.

7 participants