Closed Bug 1015521 Opened 5 years ago Closed 5 years ago

Make translation provider use available API key

Categories

(Firefox :: Translation, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: Felipe, Assigned: mikedeboer)

References

Details

(Whiteboard: p=2 s=it-32c-31a-30b.3 [qa-])

Attachments

(1 file, 1 obsolete file)

In BingTranslator.jsm, BingTokenManager should use the client id and api key available.

It should:
 - check if hidden prefs exist:
    browser.translation.bing.clientIdOverride and/or
    browser.translation.bing.apiKeyOverride

 - if they exist, use them
 - if they don't exist, use BING_API_CLIENTID and BING_API_KEY provided by nsURLFormatter.js

Both values need to be encodeURLComponent'ed to be used
Flags: firefox-backlog+
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Whiteboard: p=2 s=it-32c-31a-30b.3 [qa-]
Comment on attachment 8431571 [details] [diff] [review]
Patch v1: Bing translation service will now uses available credentials

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

The encodeURIComponent step comes before the formatURL step. So if the key comes from %BING_API_KEY% instead of the hidden pref, it won't be encoded
Attachment #8431571 - Flags: review?(felipc)
Good catch! I think this one'll do the trick.
Attachment #8431571 - Attachment is obsolete: true
Attachment #8432459 - Flags: review?(felipc)
Comment on attachment 8432459 [details] [diff] [review]
Patch v1.1: Bing translation service will now uses available credentials

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

Thanks Mike! Code looks good.  Just one suggestion for change: since the main use case is getting the %ABC% keys, and the pref is a fallback, let's rename the method to: getAuthTokenParam (or something like that), and invert the order of the function arguments (token name first, override pref second)
Attachment #8432459 - Flags: review?(felipc) → review+
Pushed to fx-team, with comments addressed: https://hg.mozilla.org/integration/fx-team/rev/7a1a3c3942bd
https://hg.mozilla.org/mozilla-central/rev/7a1a3c3942bd
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.