Closed
Bug 1345365
Opened 7 years ago
Closed 7 years ago
[Payment Request API] Support canMakePayment flow
Categories
(Core :: DOM: Web Payments, enhancement, P2)
Core
DOM: Web Payments
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: edenchuang, Assigned: edenchuang)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [WP-MVP][M3])
Attachments
(2 files, 4 obsolete files)
103.41 KB,
patch
|
edenchuang
:
review+
|
Details | Diff | Splinter Review |
9.94 KB,
patch
|
edenchuang
:
review+
|
Details | Diff | Splinter Review |
This bug is used for tracking canMakePayment flow implementation, which is a part of Payment Request API.
Assignee | ||
Updated•7 years ago
|
Comment 1•7 years ago
|
||
Hi Marcos, per my understanding this is planned to have in the next releases, so P2 is the right priority, right? :)
Component: DOM → DOM: Device Interfaces
Flags: needinfo?(mcaceres)
Comment 2•7 years ago
|
||
Yep, P2 is fine. However, I can't say with any certainty which release we will target yet.
Flags: needinfo?(mcaceres)
Updated•7 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Priority: P3 → P2
Updated•7 years ago
|
Component: DOM: Device Interfaces → DOM: Web Payments
Updated•7 years ago
|
Whiteboard: [WP-MVP][M3]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → echuang
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=efe7ffc9daa6f82f2aed0c883a84e08e4d83f994
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d01cf86b14e7221a29534410f302c92aa7c4a4e
Assignee | ||
Comment 5•7 years ago
|
||
Since the canMakePayment(), abort() and show() method implementation is very similar, I merge the implementation into one patch.
Attachment #8873684 -
Flags: review?(amarchesini)
Assignee | ||
Comment 6•7 years ago
|
||
Fix some nits from previous patch.
Attachment #8873684 -
Attachment is obsolete: true
Attachment #8873684 -
Flags: review?(amarchesini)
Attachment #8876578 -
Flags: review?(amarchesini)
Assignee | ||
Comment 7•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f93feb62259c17b2b1a185a325df9d44fa5a74e
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8876578 [details] [diff] [review] Bug 1345365 - PaymentRequest API canMakePayment(), abort() and show() implementation. r?baku canceling review to update the new one for review.
Attachment #8876578 -
Flags: review?(amarchesini)
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8876578 -
Attachment is obsolete: true
Attachment #8877023 -
Flags: review?(amarchesini)
Assignee | ||
Comment 10•7 years ago
|
||
This patch is the mochitests for PaymentRequest API canMakePayment() and abort(). And following is the try result with all attached patches https://treeherder.mozilla.org/#/jobs?repo=try&revision=f910d2d6bf50bb051d086ceef416791baeefa685
Attachment #8877025 -
Flags: review?(amarchesini)
Comment 11•7 years ago
|
||
Comment on attachment 8877023 [details] [diff] [review] Bug 1345365 - PaymentRequest API canMakePayment(), abort() and show() implementation. r?baku Review of attachment 8877023 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/payments/PaymentActionResponse.cpp @@ +29,5 @@ > +} > + > +NS_IMETHODIMP > +PaymentActionResponse::GetType(uint32_t* aType) > +{ pointer check? @@ +47,5 @@ > +} > + > +NS_IMETHODIMP > +PaymentCanMakeActionResponse::GetResult(bool* aResult) > +{ pointer check? @@ +135,5 @@ > +} > + > +NS_IMETHODIMP > +PaymentShowActionResponse::IsAccepted(bool* aIsAccepted) > +{ pointer check? @@ +153,5 @@ > +} > + > +NS_IMETHODIMP > +PaymentAbortActionResponse::GetAbortStatus(uint32_t* aAbortStatus) > +{ pointer check? @@ +169,5 @@ > +} > + > +NS_IMETHODIMP > +PaymentAbortActionResponse::IsSucceeded(bool* aIsSucceeded) > +{ pointer check? @@ +187,5 @@ > +} > + > +nsresult > +PaymentCompleteActionResponse::Init(const nsAString& aRequestId, > + const uint32_t aCompleteStatus) indentation @@ +196,5 @@ > +} > + > +nsresult > +PaymentCompleteActionResponse::GetCompleteStatus(uint32_t* aCompleteStatus) > +{ check the pointer? @@ +203,5 @@ > +} > + > +nsresult > +PaymentCompleteActionResponse::IsCompleted(bool* aIsCompleted) > +{ check the pointer? ::: dom/payments/PaymentRequest.cpp @@ +335,5 @@ > + nsresult aRv) > +{ > + MOZ_ASSERT(mAcceptPromise); > + MOZ_ASSERT(ReadyForUpdate()); > + should you check the state here? @@ +398,5 @@ > + > +void > +PaymentRequest::RespondAbortPayment(bool aSuccess) > +{ > + MOZ_ASSERT(mAbortPromise); You should check the state here? ::: dom/payments/PaymentRequestService.cpp @@ +322,5 @@ > +NS_IMETHODIMP > +PaymentRequestService::RespondPayment(nsIPaymentActionResponse* aResponse) > +{ > + NS_ENSURE_ARG_POINTER(aResponse); > + nsString requestId; nsAutoString ? ::: dom/payments/PaymentResponse.h @@ +17,5 @@ > +class PaymentAddress; > +class Promise; > + > +} // namespace dom > +} // namespace mozilla remove from line 21 to 25. ::: dom/payments/ipc/PPaymentRequest.ipdl @@ +113,5 @@ > + > +struct IPCPaymentShowActionResponse > +{ > + nsString requestId; > + bool isAccepted; don't do this indentation. bool isAccepted; ::: dom/payments/ipc/PaymentRequestChild.cpp @@ +32,5 @@ > +{ > + if (!mActorAlive) { > + return IPC_FAIL_NO_REASON(this); > + } > + IPCPaymentActionResponse response = aResponse; Don't duplicate data: const IPCPaymenetActionResponse& response = aResponse; ::: dom/payments/ipc/PaymentRequestParent.cpp @@ +37,4 @@ > nsresult rv; > switch (aRequest.type()) { > case IPCPaymentActionRequest::TIPCPaymentCreateActionRequest: { > IPCPaymentCreateActionRequest request = aRequest; change this as the following comments. @@ +84,5 @@ > + MOZ_ASSERT(action); > + break; > + } > + case IPCPaymentActionRequest::TIPCPaymentCanMakeActionRequest: { > + IPCPaymentCanMakeActionRequest request = aRequest; This duplicate data. Can you do: const IPCPaymenetCanMakeActionRequest& request = aRequest; @@ +94,5 @@ > + } > + break; > + } > + case IPCPaymentActionRequest::TIPCPaymentShowActionRequest: { > + IPCPaymentShowActionRequest request = aRequest; same here. @@ +104,5 @@ > + } > + break; > + } > + case IPCPaymentActionRequest::TIPCPaymentAbortActionRequest: { > + IPCPaymentAbortActionRequest request = aRequest; and here. @@ +182,5 @@ > + nsCOMPtr<nsIPaymentShowActionResponse> response = > + do_QueryInterface(aResponse); > + MOZ_ASSERT(response); > + bool isAccepted; > + nsString methodName; nsAutoString for all of these?
Attachment #8877023 -
Flags: review?(amarchesini) → review+
Updated•7 years ago
|
Attachment #8877025 -
Flags: review?(amarchesini) → review+
Updated•7 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 12•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dabbdebc569856e016a41948b396cca639c122f
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8877023 -
Attachment is obsolete: true
Attachment #8877571 -
Flags: review+
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8877025 -
Attachment is obsolete: true
Attachment #8877573 -
Flags: review+
Assignee | ||
Comment 15•7 years ago
|
||
[Feature/regressing bug #]: Bug 1345365 [User impact if declined]: No impact. [Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dabbdebc569856e016a41948b396cca639c122f [Risks and why]: No risk, need preference to turn on the feature. [String/UUID change made/needed]: No
Keywords: checkin-needed
Comment 16•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bba1dee06769 PaymentRequest API canMakePayment(), abort() and show() implementation. r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/0ea7a5deacf6 Mochitest for PaymentRequest API canMakePayment() and abort(). r=baku
Keywords: checkin-needed
This appears to have caused a frequent intermittent failure: https://treeherder.mozilla.org/logviewer.html#?job_id=107223293&repo=mozilla-inbound Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/cb10c0fd0c6a585a13f1796f17fa77aa0cc6202c
Flags: needinfo?(echuang)
If it helps, this shows the frequency of the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&fromchange=980a145fce8634ddb92193f7ce278e1916f9eabb&noautoclassify&filter-searchStr=e01cd1c388e31c27eb40afa6a41221dec482fc30&tochange=cb10c0fd0c6a585a13f1796f17fa77aa0cc6202c&group_state=expanded&selectedJob=107222423
Assignee | ||
Comment 19•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bac1eb8459104dd398f655d46f4c197d763d9ca
Assignee | ||
Comment 20•7 years ago
|
||
I guess the problem is not related my patches. Even my patches is backed out, the intermittent failure still happens frequently. Now bug 1373346 is tracking the intermittent issues. and I would like to set checkin needed for these patches again.
Flags: needinfo?(echuang)
Keywords: checkin-needed
Comment 21•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe227c1c67e5 PaymentRequest API canMakePayment(), abort() and show() implementation. r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/b1fcadce1fed Mochitest for PaymentRequest API canMakePayment() and abort(). r=baku
Keywords: checkin-needed
Comment 22•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fe227c1c67e5 https://hg.mozilla.org/mozilla-central/rev/b1fcadce1fed
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 24•7 years ago
|
||
I've updated the browser support info for these pages: https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/canMakePayment https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/abort https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/show
Keywords: dev-doc-needed → dev-doc-complete
Comment 25•7 years ago
|
||
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #24) > I've updated the browser support info for these pages: > > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/ > canMakePayment > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/abort > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/show The Specifications link in above pages are either invalid (show) or incorrect (canMakePaymenta & abort).
Comment 26•7 years ago
|
||
(In reply to Ben Tian [:btian] from comment #25) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #24) > > I've updated the browser support info for these pages: > > > > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/ > > canMakePayment > > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/abort > > https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/show > > The Specifications link in above pages are either invalid (show) or > incorrect (canMakePaymenta & abort). I've updated the URL fragments in the articles, and updated the Payment Request API root URL in the MDN JSON file used to generate the links. Once this PR is accepted, the URLs will be correct on the page.
Comment 27•7 years ago
|
||
I've also made sure the browser compat info, etc. is up to date for PaymentResponse: https://developer.mozilla.org/en-US/docs/Web/API/PaymentResponse (I didn't realize it was part of this bug until now ;-) )
You need to log in
before you can comment on or make changes to this bug.
Description
•