search

Aaronius / penpal

A promise-based library for securely communicating with iframes via postMessage.
MIT License
389 stars 56 forks source link

Fixed edge case issue in connectToChild handleMessage #21

Closed MKRazz closed 5 years ago

MKRazz commented 5 years ago

I don't fully understand what is happening in my edge case, but this seems to solve the problem.

I used Penpal on a few sites and it worked fine, but on another site it wasn't responding to the handshake. There are too many differences between the projects to know the exact cause of the issue, but here were my observations:

(All tested in chrome) Originally, iframe.contentWindow would return a Window object. Later, it returns a global object. I'm not entirely sure why, my guess would be related to webpack.

On the sites that Penpal worked fine, logging child in handleMessage would return the updated global object, and the event.source would also return it, so everything was fine.

On the site Penpal didn't work, logging child in handleMessage would return the original Window object, but the event.source was returning the global object, so the event.source === child condition would fail. But, logging iframe.contentWindow in handleMessage did return the global object, so it allowed the conditional to pass.

So to solve my issue, I moved the child definition inside the handleMessage event so that in both cases it is getting the correct source object for the child. I'm not sure if there are any implications to this change that I'm not seeing, but everything seems to be going well so far.

Aaronius commented 5 years ago

Hi @MKRazz. Thanks for taking the time to open this pull request and explain your problem. When I get some time, I'll try to dig around and see if I can figure what might be going on in your case. Do you happen to have a public site that exhibits this behavior or some way I can reproduce it? That would go a long way. I'm hesitant to make any change without knowing why the change needs to be made, so I'll probably want to get to the root of the problem first.

Aaronius commented 5 years ago

@MKRazz Thanks again for reporting this issue. I'd love to get it solved, but I still don't understand what's going on in your case. I would also guess Webpack might be getting involved somehow, but I don't know. I would appreciate it if you could provide something reproducible, but understand it can be difficult and time-consuming. If you can, I can dig into it and see what's going on. Until then, I don't feel comfortable changing anything without know why the change needs to be made. I hope that's understandable.