When our nsIPaymentUIService implementation's `showPayment` method is called, open paymentRequest.xhtml as a dialog. When `abortPayment` is called while a PaymentRequest is in progress, close the appropriate dialog (using the requestId).

Do not run this snippet with `request` and `request2` at the same time. This snippet might not work if you run it with the new API specs; notably, `supportedMethods` is now simply a string.

Here's a preview of the current behaviour: http://g.recordit.co/VI8f1wv8AI.gif We're now able to hook onto the `showPayment` and `abortPayment` methods from the js. The gif presents a first PaymentRequest shown then aborted, and a 2nd PaymentRequest shown then aborted (both aborted inside `setTimeout`s. A next step would be to be able to abort the PaymentRequest from the UI itself - for example, a button "abort" in the UI triggers the abort.

(In reply to Jonathan Guillotte-Blouin [:jonathanGB] from comment #3) > Here's a preview of the current behaviour: > http://g.recordit.co/VI8f1wv8AI.gif > > We're now able to hook onto the `showPayment` and `abortPayment` methods > from the js. The gif presents a first PaymentRequest shown then aborted, and > a 2nd PaymentRequest shown then aborted (both aborted inside `setTimeout`s. Nice! > A next step would be to be able to abort the PaymentRequest from the UI > itself - for example, a button "abort" in the UI triggers the abort. I think that can be a separate bug though. For this bug I think we should add a mochitest-browser-chrome test to test the two methods and that's all. In a separate commit you could add console.jsm usage with the maxLogLevelPref if you think it will be useful to leave some debug logging in for development.

Comment on attachment 8887745 [details] Bug 1381186 - add show/abort chrome test. https://reviewboard.mozilla.org/r/158660/#review163966 ::: toolkit/components/payments/test/browser/blank_page.html:4 (Diff revision 1) > + <title>Blank page</title> > + <meta charset="utf8"> <meta charset> has to come before <title> in order for it to apply to the title. I believe you would still get a console as you have it. ::: toolkit/components/payments/test/browser/browser_show_dialog.js:3 (Diff revision 1) > +const BLANK_PAGE_URL = "https://example.com/browser/toolkit/components/" + > + "payments/test/browser/blank_page.html"; Move this to head.html ::: toolkit/components/payments/test/browser/browser_show_dialog.js:8 (Diff revision 1) > + data: { > + supportedNetworks: ["visa", "mastercard"], > + supportedTypes: ["debit"], > + }, Remove the `data` property since it's not required[1] and may cause this test to fail in the future when we actually check if storage has supported cards. [1] https://www.w3.org/TR/payment-method-basic-card/#basiccardrequest ::: toolkit/components/payments/test/browser/browser_show_dialog.js:14 (Diff revision 1) > + supportedNetworks: ["visa", "mastercard"], > + supportedTypes: ["debit"], > + }, > +}]; > +const details = { > + id: "super-store-order-123-12312", Remove the optional id property ::: toolkit/components/payments/test/browser/browser_show_dialog.js:17 (Diff revision 1) > +}]; > +const details = { > + id: "super-store-order-123-12312", > + total: { > + label: "Total due", > + amount: { currency: "USD", value: "60.00" }, // US$60.00 Remove the comment please as it's not useful ::: toolkit/components/payments/test/browser/browser_show_dialog.js:20 (Diff revision 1) > +const options = { > + requestShipping: true, > +}; Since the third argument is optional and requestShipping isn't related to this test we should leave it out to keep it as minimal/focused as possible. You can leave it available on the helper for later though. ::: toolkit/components/payments/test/browser/browser_show_dialog.js:25 (Diff revision 1) > +const options = { > + requestShipping: true, > +}; > + > +add_task(async function test_show_abort_dialog() { > + await SpecialPowers.pushPrefEnv({set: [[PREF_PAYMENT_ENABLED, true]]}); This should go in a separate setup task which is in head.js: add_task(async function setup_head() { … }); ::: toolkit/components/payments/test/browser/browser_show_dialog.js:34 (Diff revision 1) > + async({methodData, details, options}) => { > + let rq = new content.PaymentRequest(methodData, details, options); > + content.rq = rq; // assign it so we can retrieve it later > + rq.show(); > + }); Since I think many tests will need this pattern and the indenation is a bit hard to read with this inline, I think we should make this async method a helper in head.js on an object that makes it clear it's for content helpers, not running in browser-chrome: ```js /** * Common content tasks functions to be used with ContentTask.spawn. */ let ContentTasks = { async showRequest({methodData, details, options}) => { … } }; ``` ::: toolkit/components/payments/test/browser/browser_show_dialog.js:46 (Diff revision 1) > + if (win.closed || !requestId) { > + return; > + } Do bother with nicer paths in test failure scenarios. i.e. remove this ::: toolkit/components/payments/test/browser/head.js:9 (Diff revision 1) > + args: "none", > + varsIgnorePattern: "^(Cc|Ci|Cr|Cu|EXPORTED_SYMBOLS)$", > + }], > +*/ > + > +const REQUEST_ID_REGEX = /^paymentRequest-(.*)$/; I don't think you need a regex here, only to define the prefix, that way it can be used both for matching and constructing the name. ::: toolkit/components/payments/test/browser/head.js:12 (Diff revision 1) > +*/ > + > +const REQUEST_ID_REGEX = /^paymentRequest-(.*)$/; > + > +async function getDialogWindow() { > + return Services.wm.getMostRecentWindow(null); I think you should check that the window is a payment window otherwise `throw new Error("…")` ::: toolkit/components/payments/test/browser/head.js:15 (Diff revision 1) > +function getDialogRequestId(windowName) { > + let result = windowName.match(REQUEST_ID_REGEX); This function signature won't apply to the proper future widget so we should change it to be forward compatible. We should write the tests so that we can simply replace the widget and update these helpers (without touching the callers) and everything will pass still but in this case I think passing win.name is too much of an implementation detail. Maybe call this requestIdForWindow(window) and extract the name from the window inside it.

Comment on attachment 8887745 [details] Bug 1381186 - add show/abort chrome test. https://reviewboard.mozilla.org/r/158660/#review165354 ::: toolkit/components/payments/test/browser/browser.ini:8 (Diff revision 2) > + > support-files = > + blank_page.html > > [browser_request_summary.js] > +[browser_show_dialog.js] Add `skip-if = !e10s` due to bug 1365964 and add a comment to fix the failing test on try: # Bug 1365964 - Payment Request isn't implemented for non-e10s ::: toolkit/components/payments/test/browser/head.js:16 (Diff revision 2) > + let win = Services.wm.getMostRecentWindow(null); > + if (!win.name.startsWith(REQUEST_ID_PREFIX)) { > + throw new Error("most recent window is not a PaymentRequest dialog"); > + } Since the dialog window may not be opened yet and we don't have a generic way to know about that yet, we can change this to use BrowserTestUtils.waitForCondition where the condition is updating `win = Services.wm.getMostRecentWindow(null);` and returning `win.name.startsWith(REQUEST_ID_PREFIX)`. Then the `getDialogWindow` method can still `return win;` This should fix the other test failure

Comment on attachment 8887298 [details] Bug 1381186 - open/close stub dialog on (show/abort)Payment. https://reviewboard.mozilla.org/r/158100/#review165368 There are some eslint issues to fix

Comment on attachment 8887298 [details] Bug 1381186 - open/close stub dialog on (show/abort)Payment. https://reviewboard.mozilla.org/r/158100/#review166980 ::: npm-shrinkwrap.json:1 (Diff revision 3) > { > "name": "mozillaeslintsetup", > - "requires": true, > "lockfileVersion": 1, Revert this file accidentally touched.

Comment on attachment 8887298 [details] Bug 1381186 - open/close stub dialog on (show/abort)Payment. https://reviewboard.mozilla.org/r/158100/#review167042 ::: toolkit/components/payments/paymentUIService.js:50 (Diff revision 4) > + chromeWindow.openDialog(DIALOG_URL, > + `${this.REQUEST_ID_PREFIX}${requestId}`, "modal,dialog,centerscreen"); Nit: This is wrapped inconsistently ::: toolkit/components/payments/paymentUIService.js:62 (Diff revision 4) > .createInstance(Ci.nsIPaymentAbortActionResponse); > abortResponse.init(requestId, Ci.nsIPaymentActionResponse.ABORT_SUCCEEDED); > + > + let enu = Services.wm.getEnumerator(null); > + let win; > + this.log.debug(`requestId: ${requestId}`); Can you move this to the abortPayment log message so that we know what the requestId is for: `abortPayment: ${requestId}`

Comment on attachment 8887745 [details] Bug 1381186 - add show/abort chrome test. https://reviewboard.mozilla.org/r/158660/#review167046 ::: browser/installer/package-manifest.in:400 (Diff revision 4) > +#ifdef NIGHTLY_BUILD > @RESPATH@/components/payments.manifest > +@RESPATH@/components/paymentUIService.js > +#endif This needs to be rebased on m-c ::: toolkit/components/payments/test/browser/head.js:1 (Diff revision 4) > +/* eslint > + "no-unused-vars": ["error", { Can you add "use strict"; to this file please and make sure tests still pass locally.

Comment on attachment 8887745 [details] Bug 1381186 - add show/abort chrome test. https://reviewboard.mozilla.org/r/158660/#review167046 > This needs to be rebased on m-c After rebase an instruction was added to `package-manifest.in`, see interdiff 4-5

Comment on attachment 8887745 [details] Bug 1381186 - add show/abort chrome test. https://reviewboard.mozilla.org/r/158660/#review171560 ::: browser/installer/package-manifest.in:403 (Diff revision 6) > -#if defined(NIGHTLY_BUILD) && defined(MOZ_BUILD_APP_IS_BROWSER) > +#ifdef NIGHTLY_BUILD > @RESPATH@/components/payments.manifest > +@RESPATH@/components/paymentUIService.js > #endif Shouldn't the package and browser_all_files_referenced.js changes be in the other commit? Can you also rebase this on recent m-c so it can autoland?

Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/1df19277c9c1 open/close stub dialog on (show/abort)Payment. r=MattN https://hg.mozilla.org/integration/autoland/rev/4bdfc48d9639 add show/abort chrome test. r=MattN

https://hg.mozilla.org/mozilla-central/rev/1df19277c9c1 https://hg.mozilla.org/mozilla-central/rev/4bdfc48d9639