Closed Bug 815769 Opened 12 years ago Closed 12 years ago

Only require an https payment provider if the dom setting says to

Categories

(Core :: DOM: Device Interfaces, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: kumar, Assigned: ferjm)

Details

Attachments

(1 file)

navigator.mozPay() (bug 767818) currently restricts all payment providers to only those having https URLs [1]. This makes B2G development difficult against a localhost payment provider and thus shortens the feedback cycle when building features.

Let's introduce a dom setting to toggle the https check on/off. Something like dom.payment.provider.requireHttps = true ?

This pref would be for devs only.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/payment/Payment.jsm#298
s/shortens feedback cycle/delays feedback cycle/g :)
Assignee: nobody → ferjmoreno
Attached patch v1Splinter Review
Attachment #687854 - Flags: review?(fabrice)
Comment on attachment 687854 [details] [diff] [review]
v1

Review of attachment 687854 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comments addressed

::: dom/payment/Payment.jsm
@@ +47,5 @@
> +    // We first check if the preference actually exists, so we don't get an
> +    // NS_ERROR_UNEXPECTED error.
> +    if (paymentPrefs.getPrefType("skipHTTPSCheck")) {
> +      this.checkHttps = !paymentPrefs.getBoolPref("skipHTTPSCheck");
> +    }

Usually we do it that way:
let value = defaultValue;
try {
 value = prefBranch.getBoolPref("the.pref.name");
} catch(e) {}

@@ +307,5 @@
>      }
>  
>      // We only allow https for payment providers uris.
> +    if (this.checkHttps) {
> +      if (!/^https/.exec(provider.uri.toLowerCase())) {

nit: if (this.checkHttps && !/.../) {
}
Attachment #687854 - Flags: review?(fabrice) → review+
Comment on attachment 687854 [details] [diff] [review]
v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: Based on comment 0, no user impact, but developer impact
Testing completed (on m-c, etc.): Local tests.
Risk to taking this patch (and alternatives if risky): B2G only feature, and well tested locally, so very low risk.
String or UUID changes made by this patch: None.
Attachment #687854 - Flags: approval-mozilla-beta?
Attachment #687854 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/8f0067ac46f1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Thanks ferjm! This is going to tremendously speed up development time on the frontend payments UI.
Comment on attachment 687854 [details] [diff] [review]
v1

developer feature, doesn't appear to impact users on desktop/mobile, so approving for uplift.
Attachment #687854 - Flags: approval-mozilla-beta?
Attachment #687854 - Flags: approval-mozilla-beta+
Attachment #687854 - Flags: approval-mozilla-aurora?
Attachment #687854 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: