Closed Bug 1014367 Opened 6 years ago Closed 6 years ago

API key support for translation

Categories

(Firefox :: Translation, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: Felipe, Assigned: Felipe)

References

Details

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

Attachments

(1 file)

Add the support for the Bing API clientid/key tokens.

There are 3 parts to it:
 - add the build time support for including the keys
 - use the keys in the code
 - add the keyfile/mozconfig changes to production

This bug is about part 1.
Flags: firefox-backlog+
Gavin, what do you think about using the same structure as the other keys, going through nsURLFormatter like this?
Attachment #8426793 - Flags: feedback?(gavin.sharp)
Comment on attachment 8426793 [details] [diff] [review]
API key build patch

Sounds good to me.
Attachment #8426793 - Flags: feedback?(gavin.sharp) → feedback+
Attachment #8426793 - Flags: review?(ted)
Comment on attachment 8426793 [details] [diff] [review]
API key build patch

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

::: configure.in
@@ +4038,5 @@
>      MOZ_GOOGLE_API_KEY=no-google-api-key
>  fi
>  AC_SUBST(MOZ_GOOGLE_API_KEY)
>  
> +# Allow to specify a Bing API key file that contains the client ID and the

Grammar nit: "Allow specifying"

::: toolkit/components/urlformatter/Makefile.in
@@ +16,5 @@
>  	@echo '#define MOZ_GOOGLE_API_KEY $(MOZ_GOOGLE_API_KEY)' > $@
>  
> +bing_api_key:
> +	@echo '#define MOZ_BING_API_KEY $(MOZ_BING_API_KEY)' > $@
> +	@echo '#define MOZ_BING_API_CLIENTID $(MOZ_BING_API_CLIENTID)' >> $@

We should probably just create these files in configure instead. I won't make you change that since you're just following precedent.
Attachment #8426793 - Flags: review?(ted) → review+
Thanks!

> ::: configure.in
> @@ +4038,5 @@
> >      MOZ_GOOGLE_API_KEY=no-google-api-key
> >  fi
> >  AC_SUBST(MOZ_GOOGLE_API_KEY)
> >  
> > +# Allow to specify a Bing API key file that contains the client ID and the
> 
> Grammar nit: "Allow specifying"

This was just copy&paste from the two comment blocks above. I will fix the comment on the others too when landing, ok?
I actually left the other blocks untouched because it was such a small thing that I just avoided messing with the hg blame on them. I fixed the phrase on my new block.

https://hg.mozilla.org/integration/fx-team/rev/459040b37060
Added to Iteration 32.2
Status: NEW → ASSIGNED
Whiteboard: [translation] p=1 s=it-32c-31a-30b.2 [qa-]
https://hg.mozilla.org/mozilla-central/rev/459040b37060
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Status: RESOLVED → VERIFIED
Blocks: 1015521
You need to log in before you can comment on or make changes to this bug.