Skip to content

Rename WebView postMessage method to postMessageNative.#12997

Closed
soldovskii wants to merge 3 commits intofacebook:masterfrom
soldovskii:master
Closed

Rename WebView postMessage method to postMessageNative.#12997
soldovskii wants to merge 3 commits intofacebook:masterfrom
soldovskii:master

Conversation

@soldovskii
Copy link

@soldovskii soldovskii commented Mar 17, 2017

Rename WebView postMessage to postMessageNative. The old name "postMessage" breaks the functional of some scripts in WebView using cross-domain communication via postMessage. Example: https://api-maps.yandex.ru/2.1/?lang=ru_RU

Motivation (required)

The old name "postMessage" breaks the functional of some scripts in WebView using cross-domain communication via postMessage. This bug avalible if WebView has onMessage callback.

Test Plan (required)

Not working
2017-03-17 20 11 33

Working
2017-03-17 20 10 33

Send message to react-native WebView - window.postMessageNative("data", "*");

Replace original window.postMessage - bad idea;

…ssage" breaks the functional of some scripts in WebView using cross-domain communication via postMessage. Example: https://api-maps.yandex.ru/2.1/?lang=ru_RU
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-bot facebook-github-bot 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 Mar 17, 2017
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@farazs
Copy link

farazs commented Mar 30, 2017

I think something like window.rnBridge.postMessage would be preferable. It would also let you use the built-inJavascriptInterface directly on Android. The current implementation is slightly bugged.

@axten
Copy link

axten commented Apr 13, 2017

This PR is strongly needed, because many users get Error Setting onMessage on a WebView overrides existing values of window.postMessage, but a previous value was defined.
see #10865 for detailed info.
postMessage is official browser api, see MDN for info.

@Thomas101
Copy link
Contributor

Just ran into this today. Real pain :(

@mcmar
Copy link

mcmar commented Jun 8, 2017

The current implementation is completely 100% broken and unusable. Using it results in a red page that says Setting onMessage on a WebView overrides existing values of window.postMessage, but a previous value was defined. It always results in that red page, no matter what you do.

Given that context, I think we should merge this. Something is clearly better than nothing. And even nothing is better than something that's 100% broken.

@davidgross
Copy link

100% agree with @mcmar we are needing to be able to use onMessage for an app we are building.

@twairball
Copy link

twairball commented Jun 22, 2017

Current implementation breaks for webviews that load Meteor js content, which uses window.postMessage

@shergin
Copy link
Contributor

shergin commented Jun 27, 2017

I believe we can fix this without breaking backward compatibility. What if we can call window.originalPostMessage inside our overridden postMessage?

@twairball
Copy link

window.postMessage is a browser API. The subject issues are due to RCTWebview hijacking this window function.

In the case for RN Webviews I think its safe to assume we are using postMessage to achieve some kind of bridging between webview <--> RN ?

If so, if the concern is breaking backwards compatibility, does it make sense to instead have a parallel window.postMessageNative that doesn't impact current window.postMessage?

@neerfri
Copy link

neerfri commented Jul 2, 2017

@mkonicek is this an acceptable PR to solve #10941 ?
(as you suggest in #10941 (comment))

@nazrdogan
Copy link

@twairball Did you solve your problem. I m facing same issue.

@facebook-github-bot
Copy link
Contributor

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

@twairball
Copy link

@nazrdogan the proposed changes in this PR work. I made my own fork for current version react-native (warning; lots of work) and added similar changes for renaming postMessage to postMessageNative

@stale
Copy link

stale bot commented Oct 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 13, 2017
@stale stale bot closed this Oct 20, 2017
@henrylearn2rock
Copy link

This should definitely not be closed.

@patientplatypus
Copy link

Yeah, so this issue (see here: #10865) was brought up in 2016 and I'm still getting it in production. And a bot automatically closes a PR to fix this? Get it together you guys.

@ohtangza
Copy link

This PR is something we MUST have. Currently, it conflicts several web SDKs including Google's, which relies on window.postMessage.

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. JavaScript Stale There has been a lack of activity on this issue and it may be closed soon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.