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

payment-manifest.json and canHandle function #83

Closed
chackett opened this issue Jan 10, 2017 · 6 comments
Closed

payment-manifest.json and canHandle function #83

chackett opened this issue Jan 10, 2017 · 6 comments

Comments

@chackett
Copy link

Hello,

It appears as though payment-manifest.json is a file containing json with information relevant to a payment app. My understanding of JSON is that it is a data interchange format and not a place where functionality is defined, as is the case with the canHandle field.

In it's current form the manifest file is not valid JSON and I would expect the contents of a .json file to contain valid JSON.

As we can see in the Registration Example There is some inline Javascript provided (Note: not JSON, see below). If we wanted to avoid duplication I would expect the parameter to setManifest() to come from the payment-manifest.json url but I think that would break.

Input from others would be great here but perhaps we could have the function encoded as a string and have the client deserialise and eval() it. I think that would be a hack though, as opposed to an acceptable solution.

Other possible solutions could be:

  • Change the extension from .json to .js to denote regular Javascript code.
  • Move the function into the actual payment app i.e. Service Worker, and expose it as a handler, similar to paymentrequest. I have a feeling the function is defined in the manifest so that it can be executed even if the app is not installed though.

JSON seems to be somewhat interoperable with Javascript meaning you can drop some JSON into Javascript source code and it will be interpreted as an object. You can even augment the object and define functionality, but when that occurs it is now Javascript code and no longer the JSON data interchange format.

@rsolomakhin
Copy link
Collaborator

I favor the approach of moving the canHandle() function into the service worker as an event, similar to "intall" or "paymentRequest" event. How about "canHandlePaymentRequest" event? I don't see the value of calling canHandle() without installing the service worker. Does anyone else see a benefit to it?

@adamroach
Copy link
Contributor

@chackett -- I think there's some confusion here. While payment method manifests are represented as files, payment app manifests are not. Payment app manifests only exist in JavaScript, and are passed to paymentAppManager.setManifest() on the Service Worker registration object.

@rsolomakhin -- The primary (but I don't think only) issue with making this a normal event on the service worker is that the canHandle() code needs to be called in a carefully isolated context to prevent exfiltration of payment information to payment apps that are not selected by the user.

@ianbjacobs
Copy link
Contributor

@adamroach,

Regarding payment app manifest files not being required (at this time), the spec in section 4.4 says: "This specification does not require that the payment manifest be independently addressable (e.g., in a file on the Web)."

(That is a recent edit.)

It sounds like Rouslan and Adam are in agreement and no change is required to canHandle() from their perspective.

Conor, do you think we can do more in the spec to make clear that no file is (currently) required?

(Also, we realized today that the registration example uses quotes in consistently; we will remove those quotation marks in the next round of edits.)

@chackett
Copy link
Author

Ok, if it is the case that a file is not required that I guess my concern is irrelevant.

I guess it is just a bit confusing if the setManifest() function includes the same kind of data as contained in the manifest file. That said, it looks like the manifest file has been removed from the spec.

Considering the above commentary I see Adam and Rouslan are in agreement as do I.

So for now the manifest is JS code and if an implementor wants to create a manifest file somewhere that is up to then.

@chackett
Copy link
Author

Also, in response to @ianbjacobs - When I see references to files in a web context, I would (possibly incorrectly) assume that these are accessible files.

Perhaps in section Section 5.1 - Payment App Identifier, the reference to file could be changed to object and from now manifest is an object as opposed to a file.

@ianbjacobs
Copy link
Contributor

I am closing this issue in favor of #96; see expectation as of today's payment app task force call:
#96 (comment)

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

No branches or pull requests

4 participants