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

Implement a postMessage function and an onMessage event for webviews … #9762

Closed
wants to merge 14 commits into from
Closed

Implement a postMessage function and an onMessage event for webviews … #9762

wants to merge 14 commits into from

Conversation

jacobp100
Copy link
Contributor

@jacobp100 jacobp100 commented Sep 6, 2016

JS API very similar to web workers and node's child process.

Work has been done by somebody else for the Android implementation over at #7020, so we'd need to have these in sync before anything gets merged.

I've made a prop messagingEnabled to be more explicit about creating globals—it might be sufficient to just check for an onMessage handler though.

screen shot 2016-09-06 at 10 28 23

@ghost
Copy link

ghost commented Sep 6, 2016

By analyzing the blame information on this pull request, we identified @spicyj and @caabernathy to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Sep 6, 2016
constructor() {
super();

this.state = { messagesReceivedFromWebView: 0 };

Choose a reason for hiding this comment

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

object literal This type is incompatible with undefined. Did you forget to declare type parameter State of property Component?

@ghost
Copy link

ghost commented Sep 6, 2016

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016
return (
<View style={[styles.container, { height: 200 }]}>
<View style={styles.container}>
<Text>Messages received from web view: {messagesReceivedFromWebView}</Text>

Choose a reason for hiding this comment

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

react/no-string-refs: Using this.refs is deprecated.

@ghost
Copy link

ghost commented Sep 6, 2016

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016
@facebook-github-bot
Copy link
Contributor

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016

onMessage = () => this.setState(state => ({
messagesReceivedFromWebView: state.messagesReceivedFromWebView + 1,
}))

Choose a reason for hiding this comment

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

call of method postMessage Method cannot be called on possibly null value null

@ghost
Copy link

ghost commented Sep 6, 2016

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016
@@ -217,6 +217,49 @@ class ScaledWebView extends React.Component {
}
}

class MessagingTest extends React.Component {
webview = null

Choose a reason for hiding this comment

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

curly: Expected { after 'if' condition.

@ghost
Copy link

ghost commented Sep 6, 2016

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016
@ghost
Copy link

ghost commented Sep 6, 2016

@jacobp100 updated the pull request - view changes

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 6, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Sep 8, 2016

Thanks for the PR! It's very large so might take some time to review. I asked about opinions here as I don't use WebView: https://www.facebook.com/groups/reactnativeoss/permalink/1588566434773318

@mkonicek
Copy link
Contributor

mkonicek commented Sep 8, 2016

@jacobp100 If we go ahead with this approach it will be possible to implement the exact same API on Android, correct? Would you be up for doing that after or do you know anyone who would? :)

@mkonicek mkonicek assigned majak and unassigned majak Sep 8, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Sep 8, 2016

Let's wait if someone with iOS experience jumps in to review - I have little.

@fungilation
Copy link

I'm looking at this in 0.37. I'm using
https://github.com/alinz/react-native-webview-bridge. This looks like a
complete replacement?

On Tue, Nov 8, 2016 at 5:32 AM David Perrenoud [email protected]
wrote:

Now available in React Native v0.37.0
https://github.com/facebook/react-native/releases/tag/v0.37.0!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9762 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADoJSpkxcZVc6m-ozE01bXJ7TEgkuVvbks5q8HnmgaJpZM4J1o5Z
.

@jacobp100
Copy link
Contributor Author

You’ll have to change the webview’s JS a little, but otherwise it works the same.

@bengoism
Copy link

I keep getting the following error:

"Setting onMessage on a WebView overrides existing values of window.postMessage, but a previous value was defined".

How do I handle the scenario where the visited webpage already has a declared a window.postMessage function?

@mlostekk
Copy link

@jacobp100 can you explain which JS changes have to be made?

@jacobp100
Copy link
Contributor Author

jacobp100 commented Nov 10, 2016

You should be able to use injectedJavaScript=“window.postMessage = null”.

@mlostekk
Copy link

I am trying to re-implement this example here

https://facebook.github.io/react-native/docs/webview.html

but i cannot find the file

messagingtest.html

from

source={require('./messagingtest.html')}

Can you post the content here?

@bengoism
Copy link

@jacobp100: But after assigning null to window.postMessage I get the following error when trying to send a message window.postMessage is not a function, window.postMessage is null .. which kinda makes sense since my injected javscript in short looks like this

window.postMessage = null;
...
window.postMessage("hello");

@bengoism
Copy link

@mlostekk
https://github.com/facebook/react-native/blob/abb8ea3aea686e2cd881a61fbc66d137857ef422/Examples/UIExplorer/js/messagingtest.html

@jacobp100
Copy link
Contributor Author

jacobp100 commented Nov 10, 2016

@bengoism

window.postMessage = null;
setTimeout(() => { window.postMessage(‘hello’); });

It might be worth revisiting this. My view was just that we should override the value, and document that behaviour.

@bengoism
Copy link

@mlostekk:

Basically you should change the following on the client

window.WebViewBridge.send("hello"); 
to 
window.postMessage("hello")
  window.WebViewBridge.onMessage = function(message) {
    alert(message);
  }
  to 
  document.addEventListener('message', function(event){
    alert(event.data)
  })

@fungilation
Copy link

fungilation commented Nov 15, 2016

I'm constantly getting:
Setting onMessage on a WebView overrides existing values of window.postMessage, but a previous value was defined

I have window.postMessage = null; as the first line in WebView.injectedJavaScript. The error shows up after form submission and page reloads. Initial page load is fine in the WebView.

Is there a way around this, or is this a bug? The error seems self contradictory anyway. If there is a previous value of postMessage, it would/should be overridden as the error says. But if it is, why is there the error in the first place if postMessage is simply replaced?

@malinthe
Copy link

I'm also constantly getting the Setting onMessage on a WebView overrides existing values of window.postMessage, but a previous value was defined error message, but this only happens when there's an iframe embed in the html view. Any idea why?

@jacobp100
Copy link
Contributor Author

See #10941

@fungilation
Copy link

This is better. Though why overlap and break pages that already use window.postMessage, when RN can use a different name? Say, window.rnPostMessage. Then this issue of namespace collision is sidestepped, and original webpages that uses window.postMessage can load on page without breakage.

Otherwise, we could inject JS to run at precise the right moment (right after postMessage got moved to originalPostMessage), just to undo this RN behaviour by:

window.rnPostMessage = window.postMessage
window.postMessage = window.originalPostMessage

This would seem counter-productive at best, if not prone to breakage still as the above would need to be run at precisely the right moment after RN's replacement of window.postMessage? Original page's code would break if it calls window.postMessage that's RN's replacement instead of the real original.

@satya164
Copy link
Contributor

Since people are having issues now, how about revisiting the decision and doing something like this?

window.postMessage = function(message, targetOrigin, transfer) {
  if (targetOrigin != 'self') {
    window.originalPostMessage(message, targetOrigin, transfer);
  }
  bridge.postMessage(message);
}

i.e. always requiring targetOrigin to be specified. At least it'll not be hacky like current workarounds.

@satya164
Copy link
Contributor

satya164 commented Nov 19, 2016

Though why overlap and break pages that already use window.postMessage, when RN can use a different name

#9762 (comment)

We don't have to break pages really. It's just the current implementation.

@mkonicek
Copy link
Contributor

Can anyone running into issues submit a PR with a fix? We don't use this API internally so can't provide much feedback.

@jacobp100
Copy link
Contributor Author

Can all issues be added to #10941, and this PR be closed from further comments?

facebook-github-bot pushed a commit that referenced this pull request Jan 7, 2017
Summary:
Currently, < WebView > allows you to pass JS to execute within the view. This works great, but there currently is not a way to execute JS after the page is loaded. We needed this for our app.

We noticed that the WebView had messaging support added (see #9762) . Initially, this seemed like more than enough functionality for our use case - just write a function that's injected on initial load that accepts a message with JS, and `eval()` it. However, this broke once we realized that Content Security Policy can block the use of eval on pages. The native methods iOS provide to inject JS allow you to inject JS without CSP interfering. So, we just wrapped the native methods on iOS (and later Android) and it worked for our use case. The method injectJavaScript was born.

Now, after I wrote this code, I realized that #8798 exists and hadn't been merged because of a lack of tests. I commend what was done in #8798 as it sorely solves a problem (injecting JS after the initial load) and has more features than what I'
Closes #11358

Differential Revision: D4390425

fbshipit-source-id: 02813127f8cf60fd84229cb26eeea7f8922d03b3
gitjake pushed a commit to gitjake/react-native that referenced this pull request Jan 20, 2017
Summary:
Currently, < WebView > allows you to pass JS to execute within the view. This works great, but there currently is not a way to execute JS after the page is loaded. We needed this for our app.

We noticed that the WebView had messaging support added (see facebook#9762) . Initially, this seemed like more than enough functionality for our use case - just write a function that's injected on initial load that accepts a message with JS, and `eval()` it. However, this broke once we realized that Content Security Policy can block the use of eval on pages. The native methods iOS provide to inject JS allow you to inject JS without CSP interfering. So, we just wrapped the native methods on iOS (and later Android) and it worked for our use case. The method injectJavaScript was born.

Now, after I wrote this code, I realized that facebook#8798 exists and hadn't been merged because of a lack of tests. I commend what was done in facebook#8798 as it sorely solves a problem (injecting JS after the initial load) and has more features than what I'
Closes facebook#11358

Differential Revision: D4390425

fbshipit-source-id: 02813127f8cf60fd84229cb26eeea7f8922d03b3
atticoos pushed a commit to robinpowered/react-native that referenced this pull request Sep 21, 2017
Implement a postMessage function and an onMessage event for webviews …

Summary:
JS API very similar to web workers and node's child process.

Work has been done by somebody else for the Android implementation over at facebook#7020, so we'd need to have these in sync before anything gets merged.

I've made a prop `messagingEnabled` to be more explicit about creating globals—it might be sufficient to just check for an onMessage handler though.

![screen shot 2016-09-06 at 10 28 23](https://cloud.githubusercontent.com/assets/7275322/18268669/b1a12348-741c-11e6-91a1-ad39d5a8bc03.png)
Closes facebook#9762

Differential Revision: D4008260

fbshipit-source-id: 84b1afafbc0ab1edc3dfbf1a8fb870218e171a4c
eritter pushed a commit to eritter/react-native that referenced this pull request Sep 11, 2019
Summary:
JS API very similar to web workers and node's child process.

Work has been done by somebody else for the Android implementation over at facebook#7020, so we'd need to have these in sync before anything gets merged.

I've made a prop `messagingEnabled` to be more explicit about creating globals—it might be sufficient to just check for an onMessage handler though.

![screen shot 2016-09-06 at 10 28 23](https://cloud.githubusercontent.com/assets/7275322/18268669/b1a12348-741c-11e6-91a1-ad39d5a8bc03.png)
Closes facebook#9762

Differential Revision: D4008260

fbshipit-source-id: 84b1afafbc0ab1edc3dfbf1a8fb870218e171a4c
eritter pushed a commit to eritter/react-native that referenced this pull request Sep 11, 2019
Summary:
JS API very similar to web workers and node's child process.

Work has been done by somebody else for the Android implementation over at facebook#7020, so we'd need to have these in sync before anything gets merged.

I've made a prop `messagingEnabled` to be more explicit about creating globals—it might be sufficient to just check for an onMessage handler though.

![screen shot 2016-09-06 at 10 28 23](https://cloud.githubusercontent.com/assets/7275322/18268669/b1a12348-741c-11e6-91a1-ad39d5a8bc03.png)
Closes facebook#9762

Differential Revision: D4008260

fbshipit-source-id: 84b1afafbc0ab1edc3dfbf1a8fb870218e171a4c
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.