Closed
Bug 1446203
Opened 6 years ago
Closed 6 years ago
Basic Payment Request Shipping Address Add/Edit page
Categories
(Firefox :: WebPayments UI, enhancement, P1)
Firefox
WebPayments UI
Tracking
()
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: MattN, Assigned: jaws)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: [webpayments])
Attachments
(1 file, 1 obsolete file)
Use the factored out address form code from bug 1427954 to implement the basic add/edit address page.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Updated•6 years ago
|
Priority: P2 → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8968654 [details] Bug 1446203 - Basic Payment Request Shipping Address Add/Edit page. https://reviewboard.mozilla.org/r/237336/#review244044 ::: browser/extensions/formautofill/content/autofillEditForms.js:118 (Diff revision 2) > - const {addressLevel1Label, postalCodeLabel, fieldsOrder} = this.getFormFormat(country); > - this._elements.addressLevel1Label.dataset.localization = addressLevel1Label; > - this._elements.postalCodeLabel.dataset.localization = postalCodeLabel; > - this.arrangeFields(fieldsOrder); > + // const {addressLevel1Label, postalCodeLabel, fieldsOrder} = this.getFormFormat(country); > + // this._elements.addressLevel1Label.dataset.localization = addressLevel1Label; > + // this._elements.postalCodeLabel.dataset.localization = postalCodeLabel; > + // this.arrangeFields(fieldsOrder); These are commented out due to the following errors: > [JavaScript Warning: "Security wrapper denied access to property "addressLevel1Label" on privileged Javascript object. Support for exposing privileged objects to untrusted content via __exposedProps__ has been removed - use WebIDL bindings or Components.utils.cloneInto instead. Note that only the first denied property access from a given global object will be reported." {file: "resource://payments/formautofill/autofillEditForms.js" line: 118}] > JavaScript error: resource://payments/formautofill/autofillEditForms.js, line 134: TypeError: fieldsOrder is undefined Do you see what I am doing wrong in paymentDialogFrameScript.js that is not letting autofillEditForms.js work as intended? ::: toolkit/components/payments/content/paymentDialogFrameScript.js:72 (Diff revision 2) > exposeUtilityFunctions() { > let PaymentDialogUtils = { > + DEFAULT_REGION: FormAutofillUtils.DEFAULT_REGION, > + > + // WebPayment support is limited to "US" and "CA" currently. > + supportedCountries: ["US", "CA"], I didn't use FormAutofillUtils.supportedCountries because it includes DE. We could introduce a new pref for this but I didn't think that was worthwhile for the time being. ::: toolkit/components/payments/test/browser/browser_address_edit.js:39 (Diff revision 2) > + info("\n\n\n\ndocument\n\n\n\n" + > + content.document.body.outerHTML + > + "\n\n\n\nend document\n\n\n\n"); This is just here for debugging purposes. Due to the JS errors shown above, the form is missing the two <button> elements. With the code commented out the buttons get added but are missing their textContent because the commented out code is necessary for the textContent. ::: toolkit/components/payments/test/mochitest/mochitest.ini:5 (Diff revision 2) > + ../../../../../browser/extensions/formautofill/content/editAddress.xhtml > !/browser/extensions/formautofill/content/editCreditCard.xhtml > ../../../../../browser/extensions/formautofill/content/autofillEditForms.js editAddress.xhtml and autofillEditForms.js actually cannot use the ! syntax because they are not included by other test manifests.
Reporter | ||
Comment 4•6 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3) > Comment on attachment 8968654 [details] > Bug 1446203 - Basic Payment Request Shipping Address Add/Edit page. > > https://reviewboard.mozilla.org/r/237336/#review244044 > > ::: browser/extensions/formautofill/content/autofillEditForms.js:118 > (Diff revision 2) > > - const {addressLevel1Label, postalCodeLabel, fieldsOrder} = this.getFormFormat(country); > > - this._elements.addressLevel1Label.dataset.localization = addressLevel1Label; > > - this._elements.postalCodeLabel.dataset.localization = postalCodeLabel; > > - this.arrangeFields(fieldsOrder); > > + // const {addressLevel1Label, postalCodeLabel, fieldsOrder} = this.getFormFormat(country); > > + // this._elements.addressLevel1Label.dataset.localization = addressLevel1Label; > > + // this._elements.postalCodeLabel.dataset.localization = postalCodeLabel; > > + // this.arrangeFields(fieldsOrder); > > These are commented out due to the following errors: > > > [JavaScript Warning: "Security wrapper denied access to property "addressLevel1Label" on privileged Javascript object. Support for exposing privileged objects to untrusted content via __exposedProps__ has been removed - use WebIDL bindings or Components.utils.cloneInto instead. Note that only the first denied property access from a given global object will be reported." {file: "resource://payments/formautofill/autofillEditForms.js" line: 118}] > > JavaScript error: resource://payments/formautofill/autofillEditForms.js, line 134: TypeError: fieldsOrder is undefined > > Do you see what I am doing wrong in paymentDialogFrameScript.js that is not > letting autofillEditForms.js work as intended? Two issues: 1) You were calling the function in address-form.js instead of passing it in to the EditAddress constructor. This also meant that the argument wasn't being passed. 2) cloneInto doesn't automatically clone the return values of the cloned functions so you need to clone the return value if it's not a primitive… in this case we have an object. > ::: toolkit/components/payments/content/paymentDialogFrameScript.js:72 > (Diff revision 2) > > exposeUtilityFunctions() { > > let PaymentDialogUtils = { > > + DEFAULT_REGION: FormAutofillUtils.DEFAULT_REGION, > > + > > + // WebPayment support is limited to "US" and "CA" currently. > > + supportedCountries: ["US", "CA"], > > I didn't use FormAutofillUtils.supportedCountries because it includes DE. We > could introduce a new pref for this but I didn't think that was worthwhile > for the time being. We could remove DE from the pref and use FormAutofillUtils.supportedCountries since that pref only controls whether Germany shows in the country picker for Nightly at the moment (the feature is available in all countries for Nightly regardless). > ::: toolkit/components/payments/test/browser/browser_address_edit.js:39 > (Diff revision 2) > > + info("\n\n\n\ndocument\n\n\n\n" + > > + content.document.body.outerHTML + > > + "\n\n\n\nend document\n\n\n\n"); > > This is just here for debugging purposes. Due to the JS errors shown above, > the form is missing the two <button> elements. With the code commented out > the buttons get added but are missing their textContent because the > commented out code is necessary for the textContent. > > ::: toolkit/components/payments/test/mochitest/mochitest.ini:5 > (Diff revision 2) > > + ../../../../../browser/extensions/formautofill/content/editAddress.xhtml > > !/browser/extensions/formautofill/content/editCreditCard.xhtml > > ../../../../../browser/extensions/formautofill/content/autofillEditForms.js > > editAddress.xhtml and autofillEditForms.js actually cannot use the ! syntax > because they are not included by other test manifests. Oh yeah.
Reporter | ||
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8968654 [details] Bug 1446203 - Basic Payment Request Shipping Address Add/Edit page. https://reviewboard.mozilla.org/r/237336/#review244354 I'm not sure I should do a thorough review until more of the known issues are fixed. At quick glance things look good ::: toolkit/components/payments/test/mochitest/mochitest.ini:5 (Diff revision 2) > [DEFAULT] > prefs = > dom.webcomponents.customelements.enabled=false > support-files = > + ../../../../../browser/extensions/formautofill/content/editAddress.xhtml Hmm… I thought you would need to do the same as editCreditCard here and add editAddress.xhtml to the mochitest.ini in the formautofill subdirectory of this directory.
Attachment #8968654 -
Flags: review?(MattN+bmo)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8968654 [details] Bug 1446203 - Basic Payment Request Shipping Address Add/Edit page. https://reviewboard.mozilla.org/r/237336/#review245558
Attachment #8968654 -
Flags: review?(MattN+bmo) → review+
Comment 8•6 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 36f1bbeb2057158cb2b2dfc6b71af953366b6193 -d 0b8f0217bda4: rebasing 460467:36f1bbeb2057 "Bug 1446203 - Basic Payment Request Shipping Address Add/Edit page. r=MattN" (tip) merging browser/app/profile/firefox.js merging toolkit/components/payments/res/containers/basic-card-form.js and toolkit/components/payments/res/containers/address-form.js to toolkit/components/payments/res/containers/address-form.js merging toolkit/components/payments/res/containers/payment-dialog.js merging toolkit/components/payments/res/paymentRequest.xhtml merging toolkit/components/payments/test/browser/browser_card_edit.js and toolkit/components/payments/test/browser/browser_address_edit.js to toolkit/components/payments/test/browser/browser_address_edit.js merging toolkit/components/payments/test/browser/browser_card_edit.js merging toolkit/components/payments/test/mochitest/mochitest.ini warning: conflicts while merging toolkit/components/payments/res/containers/address-form.js! (edit, then use 'hg resolve --mark') warning: conflicts while merging toolkit/components/payments/res/paymentRequest.xhtml! (edit, then use 'hg resolve --mark') warning: conflicts while merging toolkit/components/payments/test/browser/browser_address_edit.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8969805 -
Attachment is obsolete: true
Comment 10•6 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5de60fc0f2da Basic Payment Request Shipping Address Add/Edit page. r=MattN
Comment 11•6 years ago
|
||
Backed out changeset 5de60fc0f2da (bug 1446203) for Mochitest failure on browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html. CLOSED TREE Log: https://treeherder.mozilla.org/logviewer.html#?job_id=175647170&repo=autoland&lineNumber=2829 INFO - TEST-PASS | browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html | Receive add storage changed event [task 2018-04-26T00:41:00.997Z] 00:41:00 INFO - expecting the chrome task finished: FormAutofillTest:CheckAddresses [task 2018-04-26T00:41:00.998Z] 00:41:00 INFO - Buffered messages finished [task 2018-04-26T00:41:00.999Z] 00:41:00 INFO - TEST-UNEXPECTED-FAIL | browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html | Address saved as expected [task 2018-04-26T00:41:01.000Z] 00:41:00 INFO - test_form_will_submit_without_sub_keys@browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html:72:3 [task 2018-04-26T00:41:01.001Z] 00:41:01 INFO - async*add_task/</<@SimpleTest/AddTask.js:57:34 [task 2018-04-26T00:41:01.002Z] 00:41:01 INFO - async*add_task/<@SimpleTest/AddTask.js:31:10 [task 2018-04-26T00:41:01.003Z] 00:41:01 INFO - setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:676:12 [task 2018-04-26T00:41:01.004Z] 00:41:01 INFO - add_task@SimpleTest/AddTask.js:30:7 [task 2018-04-26T00:41:01.005Z] 00:41:01 INFO - @browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html:31:1 [task 2018-04-26T00:41:01.005Z] 00:41:01 INFO - TEST-PASS | browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html | Check form submitted [task 2018-04-26T00:41:01.006Z] 00:41:01 INFO - AddTask.js | Leaving test test_form_will_submit_without_sub_keys [task 2018-04-26T00:41:01.007Z] 00:41:01 INFO - expecting the storage cleanup Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5de60fc0f2da8acd02bcff1b3ded47ea6d788d37 Backout: https://hg.mozilla.org/integration/autoland/rev/e331c3b551445f8f2c4747eb5e9777b5cbbf41c4
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jaws)
Comment 14•6 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d03421ea2005 Basic Payment Request Shipping Address Add/Edit page. r=MattN
Comment 15•6 years ago
|
||
Backed out changeset for suspicion of causing browser chrome failures on browser_address_edit.js Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=64583d3e0f6fd6ec9049bea1039948ce871f4209&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-searchStr=browser%20chrome Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=175894555&repo=autoland&lineNumber=5500 Backout link: https://hg.mozilla.org/integration/autoland/rev/61784cdfb9059f0a9849d6423eba6cc5fceaaab3
Flags: needinfo?(jaws)
Updated•6 years ago
|
Product: Toolkit → Firefox
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 19•6 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9f6c1ddff499 Basic Payment Request Shipping Address Add/Edit page. r=MattN
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jaws)
Comment 20•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9f6c1ddff499
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in
before you can comment on or make changes to this bug.
Description
•