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

Fix red screen due to window.postMessage override #22

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nacho-carnicero
Copy link

This fixes #16 according to the fix given here facebook/react-native#10865 (comment)

Important : in order to try this remember to launch the packager resetting its cache in order to be sure that the updated package gets in the bundle, so run it with:
npm start -- --reset-cache

return (
<WebView
{...this.props}
injectedJavaScript={patchPostMessageJsCode}
javaScriptEnabled={true}
Copy link
Owner

Choose a reason for hiding this comment

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

I believe this is enabled by default. Am I wrong?

Copy link
Author

Choose a reason for hiding this comment

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

Yes you're right, let me remove it.

@nacho-carnicero
Copy link
Author

Ok now it should be good, sorry for the wrong code formatting on the first commit, I didn't realize that prettier would change that before pushing. It should be fixed on the last one.

@lesnitsky
Copy link
Owner

No worries, it's good 👍
Could you also explain this fix? What is real root cause and how does this fix it

@nacho-carnicero
Copy link
Author

Well apparently setting the onMessage property on a WebView overrides window.postMessage, and the react-native developers decided to trigger an error when this occurs.

The piece of code in this PR enables to override window.postMessage without RN complaints. I was not the one developing this, I just took the fix given in the RN repo and implemented it here. If you want more info on that you can visit the issue I posted on my message above.

@ptvandi
Copy link

ptvandi commented Mar 6, 2018

Any update on this pull request? I've integrated @nacho-carnicero's fix into my project manually to resolve the window.postMessage error, but I'd love to get this resolved formally in the react-native-webview-messaging library. Thanks for the hard work, folks.

@lesnitsky
Copy link
Owner

hey @ptvandi, thanks for your feedback. I will take a closer look this evening

@lesnitsky
Copy link
Owner

@nacho-carnicero question for you: why is this patch should be evaluated inside render method of a HoC? I think it will be enough to evaluate this code just once?

@ptvandi
Copy link

ptvandi commented Mar 7, 2018

FWIW, I pulled this patch out of the render method and defined the function above the HOC declaration. No need to create the function each time the component is rendered.

@nacho-carnicero
Copy link
Author

Yes you’re right the definitions of the variables can be pulled out of the render method. I’ll push a commit tomorrow morning with the changes.

@nacho-carnicero
Copy link
Author

@R1ZZU @ptvandi I just pushed the changes, is it good for merging?

@lesnitsky
Copy link
Owner

@nacho-carnicero Not sure hoc.js is the best place for this piece of code. Maybe it worth to move this code to webview part of library? Any ideas?

@nacho-carnicero
Copy link
Author

nacho-carnicero commented Mar 12, 2018

@R1ZZU What do you think of modifying src/react-native/Webview.js to something like this?

import React from 'react';
import { WebView as NativeWebView } from 'react-native';
import { withMessaging } from './hoc';

const patchPostMessageFunction = () => {
   var originalPostMessage = window.postMessage;
   var patchedPostMessage = function(message, targetOrigin, transfer) {
      originalPostMessage(message, targetOrigin, transfer);
   };

   patchedPostMessage.toString = () => {
      return String(Object.hasOwnProperty).replace(
         'hasOwnProperty',
         'postMessage'
      );
   };
   window.postMessage = patchedPostMessage;
};

const patchPostMessageJsCode =
   '(' + String(patchPostMessageFunction) + ')();';

const WebViewWithPatchedPostMessage = ()=>{
   return(
      <NativeWebView
         injectedJavaScript={patchPostMessageJsCode}
      />
   );
};

export const WebView = withMessaging(WebViewWithPatchedPostMessage);

That way hoc.js would be unchanged.

@lesnitsky
Copy link
Owner

@nacho-carnicero this looks better, however I wonder if there is a possibility to move this part of code to webview side? Smth like src/web/remote.js sounds good, could you check if this fix will still work with this approach?

@lesnitsky
Copy link
Owner

@nacho-carnicero did you have a chance to chech if this code injected in webview code fixes the issue?

@nacho-carnicero
Copy link
Author

Sorry @R1ZZU I've been quite busy these last weeks, I didn't have a chance to look at this. Anyway since the problem comes from the react-native Webview component I doubt this would work putting it in the code corresponding to the web side. Maybe I have understood wrong what you are suggesting, could you provide some pseudo-code of what you have in mind?

@lesnitsky
Copy link
Owner

@nacho-carnicero I had no chance to dig to root cause of the issue this PR going to fix, so I'm just assuming if it also could be fixed from webview side javascript. Will this issue be fixed if this piece of code will be the first thing evaluated in webview code not with help of injectedJavaScript prop, but being the very first chunk of javascript injected in html

@nacho-carnicero
Copy link
Author

As far as I understand you would have to modify the React Native Webview component in order to avoid to use the injectedJavascript prop

@lesnitsky
Copy link
Owner

@nacho-carnicero did you have a chance to actually reproduce this issue? I'm trying to get this error and see if this code actually fixes the issue, but I just can't get original Setting onMessage on a WebView overrides existing values of window.postMessage error

@nacho-carnicero
Copy link
Author

Yes I've had it, you have to be in development mode though to encounter it (for example using the iOs simulator and clicking on the play button on XCode).

@jm90m
Copy link

jm90m commented Oct 8, 2018

@nacho-carnicero Thanks for the patch in this, I was able to just apply this patch to the normal react-native WebView to suite my needs, works fine on iOS now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting onMessage on a WebView overrides existing values of window.postMessage
4 participants