Closed Bug 1022856 Opened 10 years ago Closed 10 years ago

implement translation provider attribution


(Firefox :: Translations, defect)

Not set



Firefox 33
Tracking Status
firefox32 --- verified
firefox33 --- verified


(Reporter: Gavin, Assigned: florian)





(4 files, 1 obsolete file)

The translation provider (Bing) has attribution requirements for us of their API. We're still sorting out what those are exactly, but we'll need to add a logo/wording before launching the trial.
Flags: firefox-backlog+
(In reply to :Gavin Sharp (email from comment #0)

> we'll need to add a logo/wording before launching the trial.

Is this 'wording' something that will require new localizable strings?
Unknown yet, but if it does we can live with non-localized strings for the trial.
Assignee: nobody → felipc
Hi Felipe, are you taking this bug this iteration?
Flags: needinfo?(felipc)
Attached patch Provider attribution, WIP (obsolete) — Splinter Review
Almost there..  Took me a while fiddling with the classes to get the right footer style, but luckily the Australis work had implemented footer styles for menupopups too, not just panels.  So I was able to use them.

It needs a bit more fiddling with the right sizing, and positioning in the Preferences dialog. Note that the most important now is only the old-style Preferences window, because in-content prefs have been backed-out from Aurora.

Also need to replace the image with one that includes the "Translations by" text.

I've tested it on Mac, Win7, Win Basic, Win Classic..  Only needs more work in high-contrast mode, but that should be left for another bug.
Marco: yeah, but me and Florian will be working together on this bug. I'll provide the point estimate and QA flag tomorrow after talking with him.
Flags: needinfo?(felipc)
Added to Iteration 33.1
Whiteboard: p=0 s=33.1 [qa?]
Attached patch v2Splinter Review
Attachment #8443226 - Attachment is obsolete: true
Attachment #8443553 - Flags: review?(felipc)
Attached patch Final patchSplinter Review
Ran the image through pngcrush, changed text to German and adjusted a small detail for the Windows' CSS
Attachment #8443553 - Flags: review?(felipc) → review+
Comment on attachment 8443649 [details] [diff] [review]
Final patch

Review of attachment 8443649 [details] [diff] [review]:

r=me for the parts Felipe wrote.
Attachment #8443649 - Flags: review+
Points: --- → 5
Whiteboard: p=0 s=33.1 [qa?] → p=0 s=33.1 [qa+]
Thanks for the update Felipe.  We'll be transitioning to the new fields starting with iteration 33.2.
Points: 5 → ---
Whiteboard: p=0 s=33.1 [qa+] → p=5 s=33.1 [qa+]
Taking to match the author in the changeset Felipe pushed, but we've really both done half the work here.
Assignee: felipc → florian
Comment on attachment 8443649 [details] [diff] [review]
Final patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug is part of the automatic translation feature, which we want to A/B with a subset of Aurora 32 users.
User impact if declined: The feature won't have the service attribution required
Testing completed (on m-c, etc.): landed on inbound
Risk to taking this patch (and alternatives if risky): small, all code is only exposed to users who have the feature enabled (which is off by default)

String or IDL/UUID changes made by this patch: There's a hardcoded string that says "Translations by" in German. I talked with Gavin/Sevaan and we agreed that it's an acceptable trade-off for now as the experiment will only run for german users, and the design/string may not be final. Once it's finalized we will remove the hardcoded string and land it on central for translation.

This string is directly in the code so it won't cause any problems for the l10n tools
Attachment #8443649 - Flags: approval-mozilla-aurora?
Comment on attachment 8443649 [details] [diff] [review]
Final patch

I'm granting Aurora approval before this lands on m-c (as is the usual process) in order to support the translation A/B test as Felipe has requested. Although this code introduces a hard coded German string, as Felipe noted, the test is only for German and this code will not hit release before it is fixed (on m-c).

1. Is your intention to back out this code after the test or leave it disabled on 32 as it rides to beta and release?
2. As with your other recent bugs, please handle the Aurora landing as there is no sheriff support until next week.
Attachment #8443649 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Thanks Lawrence. I built Aurora with the patch, made sure the feature works and ran its tests. I'll also be keeping an eye on Aurora and will backout if any problem happens.

> 1. Is your intention to back out this code after the test or leave it disabled on 32 as it rides to beta and release?

No backout, we will leave it disabled and when it reaches beta we will be activating this code again through the second trial (for beta users of 3 locales). There's no intention on enabling this feature for release users in the near future.

As for the string, the idea is to remove the hardcoded version from the .js file on central and land it properly on a localizable file when we're committed to the string. And we'll see how to handle it for the 3 locales chosen for the beta trial, but that is the subject for a later bug.
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Hi Florin, can a contact be assigned for QA verification.
Iteration: --- → 33.2
Points: --- → 5
QA Whiteboard: [qa+]
Flags: needinfo?(florin.mezei)
Whiteboard: p=5 s=33.1 [qa+]
Flags: needinfo?(florin.mezei)
QA Contact: bogdan.maris
Blocks: 1030739
Tested on Windows 7 64bit, Windows 8.1 64bit Surface Pro 2, Mac OS X 10.9.3 and Ubuntu 14.04 32bit using latest Nightly and latest Aurora. Verified that the translation provider icon is displayed in Translation infobar/Options, about:preferences and Options menu (Aurora).
QA Whiteboard: [qa+] → [qa!]
Depends on: 1032139
You need to log in before you can comment on or make changes to this bug.