Closed
Bug 1022856
Opened 10 years ago
Closed 10 years ago
implement translation provider attribution
Categories
(Firefox :: Translation, defect)
Firefox
Translation
Tracking
()
People
(Reporter: Gavin, Assigned: florian)
References
()
Details
Attachments
(4 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
florian
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
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.
Updated•10 years ago
|
Flags: firefox-backlog+
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) 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?
Reporter | ||
Comment 2•10 years ago
|
||
Unknown yet, but if it does we can live with non-localized strings for the trial.
Updated•10 years ago
|
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Comment 4•10 years ago
|
||
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.
Comment 5•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8443226 -
Attachment is obsolete: true
Attachment #8443553 -
Flags: review?(felipc)
Comment 9•10 years ago
|
||
Ran the image through pngcrush, changed text to German and adjusted a small detail for the Windows' CSS
Updated•10 years ago
|
Attachment #8443553 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 10•10 years ago
|
||
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+
Comment 11•10 years ago
|
||
Points: --- → 5
Whiteboard: p=0 s=33.1 [qa?] → p=0 s=33.1 [qa+]
Comment 12•10 years ago
|
||
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+]
Assignee | ||
Comment 13•10 years ago
|
||
Taking to match the author in the changeset Felipe pushed, but we've really both done half the work here.
Assignee: felipc → florian
Comment 14•10 years ago
|
||
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 15•10 years ago
|
||
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).
Felipe:
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+
Comment 16•10 years ago
|
||
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.
Comment 17•10 years ago
|
||
status-firefox32:
--- → fixed
Comment 18•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Updated•10 years ago
|
status-firefox33:
--- → fixed
Comment 19•10 years ago
|
||
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+]
Updated•10 years ago
|
Flags: needinfo?(florin.mezei)
QA Contact: bogdan.maris
Comment 20•10 years ago
|
||
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).
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
Assignee | ||
Comment 21•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•