Closed
Bug 1434839
Opened 6 years ago
Closed 6 years ago
Rename some payments variables and files and add a button to debug the remote frame to ease development
Categories
(Firefox :: WebPayments UI, enhancement, P1)
Firefox
WebPayments UI
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: MattN, Assigned: MattN)
Details
(Whiteboard: [webpayments])
Attachments
(3 files)
* There are a few files/globals which are similar in name and even though there are in separate directories (content vs. res) and having different contexts it still causes confusion. * Currently one needs to run 4 lines of code from the docs to debug the remove unprivileged frame. Now that we have a debug frame, we should just have a button to open it instead and save a lot of time. * When developing using a file: URI (docs added), I often want to keep the debugging panel open so I added a ?debug=1 query parameter to force it to open by default. I currently waste a lot of time focusing the window and hitting the keyboard shortcut after every refresh. * Rename the `PaymentRequest` global in the unprivileged frame to `paymentRequest` so it doesn't shadow the standard window.PaymentRequest from the Payment Request spec and avoid confusing the two. I considered renaming these files and the global but `paymentRequest.cancel()` just makes sense to me.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8947371 [details] Bug 1434839 - Button to debug the remote payment frame. ?debug=1 to toggle the debug console. https://reviewboard.mozilla.org/r/217104/#review222920 Static analysis found 1 defect in this patch. - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/components/payments/content/paymentDialogWrapper.js:223 (Diff revision 1) > + if (!Services.prefs.getBoolPref("devtools.chrome.enabled", false)) { > + this.log.error("devtools.chrome.enabled must be enabled to debug the frame"); > + return; > + } > + let chromeWindow = Services.wm.getMostRecentWindow(null); > + let {gDevToolsBrowser} = ChromeUtils.import("resource://devtools/client/framework/gDevTools.jsm", Error: Line 223 exceeds the maximum line length of 100. [eslint: max-len]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8947369 [details] Bug 1434839 - Rename PaymentDialog to paymentDialogWrapper to reduce confusion. https://reviewboard.mozilla.org/r/217100/#review223026
Attachment #8947369 -
Flags: review?(jaws) → review+
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8947370 [details] Bug 1434839 - Rename 'PaymentRequest' variable to camelCase to reduce confusion with the standard API. https://reviewboard.mozilla.org/r/217102/#review223028
Attachment #8947370 -
Flags: review+
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8947371 [details] Bug 1434839 - Button to debug the remote payment frame. ?debug=1 to toggle the debug console. https://reviewboard.mozilla.org/r/217104/#review223030 ::: toolkit/components/payments/res/debugging.js:196 (Diff revision 2) > + if (window.location.protocol == "resource:") { > + // Only show the debug frame button if we're running from a resource URI > + // so it doesn't show during development over file: or http: since it won't work. > + // Note that the button still won't work if resource://payments/paymentRequest.xhtml > + // is manually loaded in a tab but will be shown. > + document.getElementById("debugFrame").hidden = false; Where does `#debugFrame` have hidden=true by default? It seems that this line is a no-op right now.
Updated•6 years ago
|
Attachment #8947369 -
Flags: review?(sfoster)
Attachment #8947370 -
Flags: review?(sfoster)
Assignee | ||
Comment 11•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8947371 [details] Bug 1434839 - Button to debug the remote payment frame. ?debug=1 to toggle the debug console. https://reviewboard.mozilla.org/r/217104/#review223030 > Where does `#debugFrame` have hidden=true by default? It seems that this line is a no-op right now. I guess you didn't see it but it's there using standard HTML format since it's not an XHTML document: ```html <button id="debugFrame" hidden>Debug frame</button> ```
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8947371 [details] Bug 1434839 - Button to debug the remote payment frame. ?debug=1 to toggle the debug console. https://reviewboard.mozilla.org/r/217104/#review223120 ::: toolkit/components/payments/res/debugging.js:196 (Diff revision 2) > + if (window.location.protocol == "resource:") { > + // Only show the debug frame button if we're running from a resource URI > + // so it doesn't show during development over file: or http: since it won't work. > + // Note that the button still won't work if resource://payments/paymentRequest.xhtml > + // is manually loaded in a tab but will be shown. > + document.getElementById("debugFrame").hidden = false; Haha, yep I see it now. I actually went back and looked for it, but I guess I should have ctrl+F looked for it.
Attachment #8947371 -
Flags: review?(jaws) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•6 years ago
|
||
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/c61a9b897dc7 Rename PaymentDialog to paymentDialogWrapper to reduce confusion. r=jaws https://hg.mozilla.org/integration/autoland/rev/236d6f06ca7f Rename 'PaymentRequest' variable to camelCase to reduce confusion with the standard API. r=jaws https://hg.mozilla.org/integration/autoland/rev/a512ed3dcbd3 Button to debug the remote payment frame. ?debug=1 to toggle the debug console. r=jaws
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c61a9b897dc7 https://hg.mozilla.org/mozilla-central/rev/236d6f06ca7f https://hg.mozilla.org/mozilla-central/rev/a512ed3dcbd3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•6 years ago
|
Priority: P3 → P1
Whiteboard: [webpayments]
Updated•6 years ago
|
Product: Toolkit → Firefox
Target Milestone: mozilla60 → Firefox 60
You need to log in
before you can comment on or make changes to this bug.
Description
•