Closed Bug 1179346 Opened 5 years ago Closed 5 years ago

Land strings for including Edge in data importer

Categories

(Firefox :: Migration, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 42
Tracking Status
firefox40 --- wontfix
firefox41 + wontfix
firefox42 --- verified

People

(Reporter: phlsa, Assigned: phlsa)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files, 3 obsolete files)

Attached image Mockup
Since 41 is already going to Aurora now, we need to land the strings needed for this feature ASAP.
If I'm not mistaken, all we'd need is the string »Microsoft Edge« (see mockup)
we also need all the strings for various options, but that should be easy
See http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/migration/migration.properties (so 1_edge, 2_edge...)

Fwiw, 41 is already Aurora, ideally we should not land string changes at this point, or we should do that like "now".
Attached patch edge-migrator-strings.patch (obsolete) — Splinter Review
Let's see if I got this right…
Attachment #8629439 - Flags: review?(mak77)
[Tracking Requested - why for this release]:
Actually, Internet Options are called "Settings" in Edge, so let's use that.
Attachment #8629439 - Attachment is obsolete: true
Attachment #8629439 - Flags: review?(mak77)
Attachment #8629440 - Flags: review?(mak77)
Comment on attachment 8629440 [details] [diff] [review]
edge-migrator-strings.patch

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

LGTM! thank you for covering lack of engineering resources.

I don't have the en-US version of Edge here (I have the italian version), so I'll trust you regarding "Settings" and "Favorites", I'm assuming you verified those terms are used as-is in their UI. Screenshots I found around seem to confirm that.

Please remember to add r=mak to the commit message.
Attachment #8629440 - Flags: review?(mak77) → review+
Assignee: nobody → philipp
Flags: firefox-backlog+
Yeah, I verified the strings :)
I don't think I have commit access (and wouldn't know how to use it if I had), so I guess checkin-needed is the thing that needs to happen now...
Keywords: checkin-needed
I think new strings are also needed in /browser/locales/en-US/chrome/browser/migration/migration.dtd
(In reply to Hector Zhao [:hectorz] from comment #7)
> I think new strings are also needed in
> /browser/locales/en-US/chrome/browser/migration/migration.dtd

oh yes, sigh, my fault.
Flags: needinfo?(philipp)
Attached patch edge-migrator-strings-dtd.patch (obsolete) — Splinter Review
Adding this as a separate patch since the other patch has already been checked in and they don't overlap.
Flags: needinfo?(philipp)
Attachment #8629952 - Flags: review?(mak77)
Comment on attachment 8629952 [details] [diff] [review]
edge-migrator-strings-dtd.patch

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

Sorry for not figuring this out earlier. Thanks to Hector that noticed the miss.
Attachment #8629952 - Flags: review?(mak77) → review+
Comment on attachment 8629952 [details] [diff] [review]
edge-migrator-strings-dtd.patch

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

::: browser/locales/en-US/chrome/browser/migration/migration.dtd
@@ +11,5 @@
>  
>  <!ENTITY importFromIE.label             "Microsoft Internet Explorer">
>  <!ENTITY importFromIE.accesskey         "M">
> +<!ENTITY importFromIE.label             "Microsoft Edge">
> +<!ENTITY importFromIE.accesskey         "E">

ehr sorry, you must change the entity name to importFromEdge....
Attachment #8629952 - Flags: review+ → review-
Attached patch edge-migrator-strings-dtd.patch (obsolete) — Splinter Review
Oops, you're right of course. Thanks for catching that!
Attachment #8629952 - Attachment is obsolete: true
Attachment #8629957 - Flags: review?(mak77) → review+
PS: you likely need to merge the 2 patches, this patch is just a rename of the previous one
OK, next try...
Attachment #8629957 - Attachment is obsolete: true
Attachment #8629963 - Flags: review?(mak77)
Attachment #8629963 - Flags: review?(mak77) → review+
 edge-migrator-strings-dtd.patch still needs checkin
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/03268e18e55c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
[Tracking Requested - why for this release]:
Re-requesting uplift to 41.
Not having the data importer work with Edge could be a real road block for users wanting to switch to Firefox after the upgrade to Windows 10.
Flags: needinfo?(lmandel)
Comment on attachment 8629440 [details] [diff] [review]
edge-migrator-strings.patch

Approval Request Comment
[Feature/regressing bug #]: see comment 19
[User impact if declined]: see comment 19
[Describe test coverage new/current, TreeHerder]: Nightly
[Risks and why]: no risk
[String/UUID change made/needed]: string changes
Attachment #8629440 - Flags: approval-mozilla-aurora?
Attachment #8629963 - Flags: approval-mozilla-aurora?
Almost all of the strings are the same. (Exceptions are product name and 'settings'.) Can we reuse strings in 41 to avoid l10n impact?

ni Pike, Flod for comment on whether we can accept these changes on Aurora.
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)
I was expecting to have a less-impact patch, tbh.

I wonder if there are locales in which Edge nomenclature actually differs from IE nomenclature? I.e., can we re-use the ie strings for anything but the product name?

Which is to say, I wouldn't mind the "Microsoft Edge" string to land, really. The rest of the strings seem to be much more regressing.

Context: Yes, we do have localizations that tell us they're done with localizing 41.
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #21)
> Almost all of the strings are the same. (Exceptions are product name and
> 'settings'.) Can we reuse strings in 41 to avoid l10n impact?

With some hacks we could probably reuse the IE strings, but still we'd need the browser name.

On the other side, it's unlikely one needs to translate "Microsoft Edge", indeed I wonder why we allow to translate browser names at all, the only reasons might be related to accesskeys.

Do you know if any locale ever changed a browser name or its accesskey here, since if that's not the case, we could just hardcode them...
(In reply to Axel Hecht [:Pike] from comment #22)
> I wonder if there are locales in which Edge nomenclature actually differs
> from IE nomenclature? I.e., can we re-use the ie strings for anything but
> the product name?

FWIW, "Settings" is in Edge, while "Internet Options" is in IE.
These likely differ.
I think that we should be able to live with an hard-coded "Microsoft Edge" and re-use IE's labels in 41, the main problem being "Settings" vs "Internet Options" (I failed to find any reference around about the change).

I checked what we have for "Internet Explorer", and besides the usual suspects (transliteration, i-Name), it seems unchanged.
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/migration/migration.properties:sourceNameIE&repo=aurora
Side note: we also have bug 1177237, we might want to coordinate any landing (if needed).
I think users won't even try to migrate from Edge for a couple months from when it's available, so likely we can survive with a sucky "Settings" translation. Also because I suspect we won't migrate settings in the first migrator implementation, we'll likely just migrate Favorites.

I just verified that the hack to reuse IE strings would also be quite simple, we already have overrides in the code.
So, if we are fine hardcoding Microsoft Edge _and_ its accesskey we can avoid the uplift (there's some risk if the accesskey we pick is already taken by some locale).
(In reply to Marco Bonardo [::mak] from comment #27)
> So, if we are fine hardcoding Microsoft Edge _and_ its accesskey we can
> avoid the uplift (there's some risk if the accesskey we pick is already
> taken by some locale).

I don't see any accesskey here though.
you should look at both attached patches
(In reply to Marco Bonardo [::mak] from comment #29)
> you should look at both attached patches

Uh, why is this bug marked as fixed with a patch not landed anywhere yet? That's what threw me off.

But yes, accesskey is going to be more painful to deal with, unless we avoid shipping one in fx41 (but I also see the aurora patch deviating more more as we speak).
So, I'm still waiting some kind of answer regarding the accesskey. Provided I'm fine with hacking most strings in Aurora, what can we do for the accesskey? Would it be acceptable to hardcode it for one version?
Flags: needinfo?(francesco.lodolo)
(In reply to Marco Bonardo [::mak] from comment #32)
> So, I'm still waiting some kind of answer regarding the accesskey. Provided
> I'm fine with hacking most strings in Aurora, what can we do for the
> accesskey? Would it be acceptable to hardcode it for one version?

If we hard-code the name (and basically don't land any new string on aurora), it's OK to hard-code the accesskey as well.

As an example I checked the "Microsoft Internet Explorer" accesskey, and there are not many changes, most of them due to transliteration or having dropped "Microsoft"
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/migration/migration.dtd:importFromIE.accesskey&repo=aurora
Flags: needinfo?(francesco.lodolo)
Marco, I have been following the discussion on this bug. I am waiting for a final decision on whether the string changes are needed or not before approving the patches for uplift. Will the patches change depending on whether the accesskey/strings are hacked? I am assuming yes. Either way, I don't think we are ready to uplift to Aurora. Please let me know if that is not the case.
Flags: needinfo?(mak77)
I think we won't uplift, we can hardcode some strings for one version since it's product names that nobody should need to localize. The only drawback is the accesskey, but it's a very edge case, so not worth it.
Flags: needinfo?(mak77)
Attachment #8629963 - Flags: approval-mozilla-aurora?
Attachment #8629440 - Flags: approval-mozilla-aurora?
Edge strings were properly landed and displayed in the importer.
Tested on Windows 10 64-bit using latest 42.0a2 Aurora, build ID: 20150902004024.
Status: RESOLVED → VERIFIED
QA Contact: cornel.ionce
You need to log in before you can comment on or make changes to this bug.