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.

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

injected javascript comments now work on android #12321

Closed
wants to merge 1 commit into from
Closed

injected javascript comments now work on android #12321

wants to merge 1 commit into from

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
@mishuagopian mishuagopian mentioned this pull request Feb 10, 2017
Closed
@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.

shergin
shergin suggested changes Aug 29, 2017
View reviewed changes
ReactAndroid/src/main/java/com/facebook/react/views/webview/ReactWebViewManager.java
@@ -273,7 +273,12 @@ public void callInjectedJavaScript() {
if (getSettings().getJavaScriptEnabled() &&
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
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

@shergin shergin shergin requested changes

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