-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Conversation
By analyzing the blame information on this pull request, we identified @spicyj and @caabernathy to be potential reviewers. |
constructor() { | ||
super(); | ||
|
||
this.state = { messagesReceivedFromWebView: 0 }; |
There was a problem hiding this comment.
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
?
@jacobp100 updated the pull request - view changes |
return ( | ||
<View style={[styles.container, { height: 200 }]}> | ||
<View style={styles.container}> | ||
<Text>Messages received from web view: {messagesReceivedFromWebView}</Text> |
There was a problem hiding this comment.
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.
@jacobp100 updated the pull request - view changes |
@jacobp100 updated the pull request - view changes |
|
||
onMessage = () => this.setState(state => ({ | ||
messagesReceivedFromWebView: state.messagesReceivedFromWebView + 1, | ||
})) |
There was a problem hiding this comment.
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
@jacobp100 updated the pull request - view changes |
@@ -217,6 +217,49 @@ class ScaledWebView extends React.Component { | |||
} | |||
} | |||
|
|||
class MessagingTest extends React.Component { | |||
webview = null |
There was a problem hiding this comment.
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.
@jacobp100 updated the pull request - view changes |
@jacobp100 updated the pull request - view changes |
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 |
@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? :) |
Let's wait if someone with iOS experience jumps in to review - I have little. |
I'm looking at this in 0.37. I'm using On Tue, Nov 8, 2016 at 5:32 AM David Perrenoud notifications@github.com
|
You’ll have to change the webview’s JS a little, but otherwise it works the same. |
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? |
@jacobp100 can you explain which JS changes have to be made? |
You should be able to use |
I am trying to re-implement this example here https://facebook.github.io/react-native/docs/webview.html but i cannot find the file
from
Can you post the content here? |
@jacobp100: But after assigning null to window.postMessage I get the following error when trying to send a message window.postMessage = null;
...
window.postMessage("hello"); |
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. |
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)
}) |
I'm constantly getting: I have 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? |
I'm also constantly getting the |
See #10941 |
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:
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. |
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 |
We don't have to break pages really. It's just the current implementation. |
Can anyone running into issues submit a PR with a fix? We don't use this API internally so can't provide much feedback. |
Can all issues be added to #10941, and this PR be closed from further comments? |
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
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
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
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
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
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.