Aaronius / penpal

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

Add the ability to apply properties to the iframe #20

Closed roly445 closed 6 years ago

roly445 commented 6 years ago

Hi, I have just started to use your library to help with the Payment Request API and for this the allowpaymentrequest property needs to be set on the iframe.

Is there a way to introduce a way of setting these properties as part the the connectToChild method?

Thanks

Aaronius commented 6 years ago

Would this work for your use case?

const connection = Penpal.connectToChild({
  ...
});

connection.iframe.allowpaymentrequest = true;

If that doesn't work for you, please let me know why. Right now you can't pass iframe properties through connectToChild, but I've been considering adding something similar.

roly445 commented 6 years ago

Maybe yes, I'm not sure the the API needs it to be present before the content of the iframe is loaded. If you had a snippet of code for you are picturing I could insert it into my local javascript and see what happens.

Aaronius commented 6 years ago

If you do it the way I mentioned above, the property should be on the iframe before the iframe's content is loaded.

As for a potential API, I've been considering something that @killix did on a fork of Penpal: https://github.com/killix/penpal/commit/f687a1f30ce804a68f1219a08d20ec25bef74dc1

I'm still trying to understand the use case(s) where this sort of API is necessary though.

roly445 commented 6 years ago

I'm doing work on the Payment Request API and so with the PCI requirements around capturing card details the code that launches and captures the card details needs to be hosted in a secure, PCI compliant environment. So the card capturing is iframe'd onto the online shop's site.

Aaronius commented 6 years ago

Okay, please let me know if this solution works for you:

const connection = Penpal.connectToChild({
  ...
});

connection.iframe.allowpaymentrequest = true;

If it does, I'll close out this ticket.

roly445 commented 6 years ago

It kinda works. I'm using typescript so I had to do connection.iframe.setAttribute('allowpaymentrequest',''), but the concept is the same. Thanks.

loganvolkers commented 6 years ago

👎 for the @killix approach.

I think that having both an appendTo and iframe option leads to a conflated API and data flow.

If you do change the API, I'd recommend a functional callback approach that also addresses some of the appendTo use cases in a minimal but powerful API.

const connection = Penpal.connectToChild({
  appendTo: (iframe) =>{
         var element = document.getElementById("foo");
         iframe.setAttribute('allowpaymentrequest','');
         iframe.style.position = "absolute";
         element.appendChild(iframe);
  }
   ...
});

I think the above approach would let people solve many of the "I need to do something with the iframe", would have a minimal API, and would solve any timing concerns with iframes being unable to be modified before they're appended to the DOM (e.g. for styling).

For reference we had to use the appendTo option with some class selectors like #foo iframe.

Aaronius commented 6 years ago

Thanks @loganvolkers. I agree with what you've described. I'm going to close this issue but I'll probably implement something similar to (or exactly) what @loganvolkers has described soon.

killix commented 6 years ago

This approach doesn't work when you want to use an existing iframe (not created by the javascript). Just #headsup I didn't make a PR because is still testing some think before.

killix commented 6 years ago

Ho and this solution:

const connection = Penpal.connectToChild({
  ...
});

connection.iframe.allowpaymentrequest = true;

look not working when attributes are scope auth attribute.

Aaronius commented 6 years ago

@killix Can you describe a case where you need to use an existing iframe?

killix commented 6 years ago

Basically, all cases when the creation/management of the iframe is managed by a component framework style (Vue, React).