Bug 769677 (metro-l10n)

[Tracking] Review localizability of metro Firefox

RESOLVED FIXED

Status

Firefox for Metro
General
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Pike, Assigned: jimm)

Tracking

Trunk
x86
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [metro-mvp][LOE:2][metro-it2][leave-open])

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
I've taken a quick glance at http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-US/.

That code isn't ready to land on central from a localizability point-of-view.

There are three obvious points that I've seen so far:

- searchplugins, including answers.com

AFAIK, this is our desktop product, it shouldn't have search engines independently of our other platforms.

- bookmarks, same as above

- TileResources, http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-US/tileresources/VisualElementsManifest.xml

no idea what that does, but that's a file that doesn't belong into locales/en-US, AFAICT. What does that file do?

I'm happy to do an actual review at some point, but I think as a first step, there should be a self-review of what of the stuff that's there is really used, and should be there. I don't think that I can be of much help in reverse engineering a platform that I don't know anything about.
(Reporter)

Comment 1

6 years ago
Data point re search: http://mxr.mozilla.org/l10n-mozilla-aurora/find?text=&string=browser%2Fsearchplugins%2F.*\.xml&regexp=1 returns 326 localized plugins that we'd need to maintain twice, if we don't build metro such that it uses the ones in browser.
(Assignee)

Updated

6 years ago
Depends on: 769718
(Assignee)

Comment 2

6 years ago
(In reply to Axel Hecht [:Pike] from comment #0)
> I've taken a quick glance at
> http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-
> US/.
> 
> That code isn't ready to land on central from a localizability point-of-view.
> 
> There are three obvious points that I've seen so far:
> 
> - searchplugins, including answers.com
> 
> AFAIK, this is our desktop product, it shouldn't have search engines
> independently of our other platforms.

So rather than store and load these from the dist/bin/app/searchplugins dir, we need to pull them from platform's dist/bin directory. 

Bug 769718

> 
> - bookmarks, same as above

Bookmarks will be different between the two apps. Maybe we can store them all in the same place and selectively install into different app dirs, not sure.

> 
> - TileResources,
> http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-
> US/tileresources/VisualElementsManifest.xml
> 

don't worry about this, this is being auto generated now using @MOZ_APP_DISPLAYNAME@ in the two strings.
(Assignee)

Updated

6 years ago
Depends on: 769490
(Assignee)

Updated

6 years ago
Blocks: 747347
(Assignee)

Updated

6 years ago
Depends on: 769724
(In reply to Jim Mathies [:jimm] from comment #2)
> (In reply to Axel Hecht [:Pike] from comment #0)
> > I've taken a quick glance at
> > http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-
> > US/.
> > 
> > That code isn't ready to land on central from a localizability point-of-view.
> > 
> > There are three obvious points that I've seen so far:
> > 
> > - searchplugins, including answers.com
> > 
> > AFAIK, this is our desktop product, it shouldn't have search engines
> > independently of our other platforms.
> 
> So rather than store and load these from the dist/bin/app/searchplugins dir,
> we need to pull them from platform's dist/bin directory. 
> 
> Bug 769718
Definitely

> 
> > 
> > - bookmarks, same as above
> 
> Bookmarks will be different between the two apps. Maybe we can store them
> all in the same place and selectively install into different app dirs, not
> sure.
The bookmarks he is referring to are the default bookmarks used when creating a profile. We should use (e.g. copy) the same default bookmarks in all cases when we create a profile.
(Assignee)

Updated

6 years ago
Severity: blocker → normal
(Reporter)

Comment 4

6 years ago
We should also package the same region.properties for search ordering etc.
(In reply to Axel Hecht [:Pike] from comment #0)
> I've taken a quick glance at
> http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/en-
> US/.
> 
> That code isn't ready to land on central from a localizability point-of-view.

I don't think this matters yet.

AIUI it's possible we might not have a polished, ready-to-ship Metro browser until well into 2013 -- we don't have a schedule yet. Current focus on initial bringup, UI prototyping, and fundamental unknowns on how Metro will interact with a Classic install... Which puts localization firmly into a P2 kind of bucket. It's not even ready for a broader group of core developers to work on.

Clearly l10n will be a growing requirement as we approach having something shippable, and I'd expect shipping requirements to be essentially the same as for today's desktop Firefox. But that need not block early progress on hooking pieces together.
(Reporter)

Comment 6

6 years ago
In our experience, at least some localizers do anything that's in a locales/en-US directory. They're probably expecting to get ahead of the game.

If you don't want to bother about how to do l10n right yet, I suggest that you don't do localizability yet. Put DTDs and stuff into content, and finalize content and build logic there, and then move the stuff over when it's ready. That saves everyone time.
(Assignee)

Comment 7

6 years ago
(In reply to Axel Hecht [:Pike] from comment #6)
> In our experience, at least some localizers do anything that's in a
> locales/en-US directory. They're probably expecting to get ahead of the game.
> 
> If you don't want to bother about how to do l10n right yet, I suggest that
> you don't do localizability yet. Put DTDs and stuff into content, and
> finalize content and build logic there, and then move the stuff over when
> it's ready. That saves everyone time.

That sounds like a great idea. I'll look at doing this when we merge the browser code over to mc.
(Reporter)

Updated

6 years ago
Depends on: 781663
(Assignee)

Updated

6 years ago
Component: General → General
Product: Firefox → Firefox for Metro

Updated

6 years ago
Whiteboard: [metro-mvp?]
(Assignee)

Comment 8

6 years ago
We have to support localization for the release of v 1.0. This work here though is related more to the mc merge. We need to get the localization setup properly in the build, or we can defer that work until the spring and yank any fennec localization logic out. Either way, this should be on the mvp list.
Whiteboard: [metro-mvp?] → [metro-mvp][LOE:2]
(Assignee)

Updated

6 years ago
Whiteboard: [metro-mvp][LOE:2] → [metro-mvp][LOE:2][metro-it2]
(Assignee)

Updated

6 years ago
Assignee: nobody → jmathies
(Assignee)

Updated

6 years ago
No longer depends on: 781663
(Assignee)

Updated

6 years ago
Summary: Review localizability of metro Firefox → [Tracking] Review localizability of metro Firefox
(Assignee)

Updated

6 years ago
Depends on: 823218
(Assignee)

Updated

6 years ago
No longer depends on: 823218
(Assignee)

Updated

6 years ago
Depends on: 823503
(Assignee)

Updated

6 years ago
Alias: elm-l10n
(Assignee)

Updated

6 years ago
Alias: elm-l10n → metro-l10n
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/projects/elm/rev/b5c0a22549e6
Whiteboard: [metro-mvp][LOE:2][metro-it2] → [metro-mvp][LOE:2][metro-it2][completed-elm]
(Assignee)

Comment 10

5 years ago
(In reply to Jim Mathies [:jimm] from comment #9)
> https://hg.mozilla.org/projects/elm/rev/b5c0a22549e6

Oops, that was the patch for bug 769490.
Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 12

5 years ago
Sorry, IIRC, there are still a good deal of files to review in the metro l10n files to make sure things work as they should.

I'm currently on travel and conference, so I'll have a hard time to dive into details, but if you want to help out, here are things we should check:

* netError.dtd/appstrings.properties: Use the mobile version or the desktop version? http://hg.mozilla.org/projects/elm/file/default/browser/metro/locales/en-US/overrides
* Are the strings we have there all used? I've come across bug 831099 in a quick search, but I'm not sure how extensive the research in that bug was?

I guess we should also be able to share the defines.inc and -l10n.js file in http://hg.mozilla.org/projects/elm/file/4c9d12cd786e/browser/metro/locales/en-US/?
Status: RESOLVED → REOPENED
Depends on: 831099
Resolution: FIXED → ---
Whiteboard: [metro-mvp][LOE:2][metro-it2][completed-elm] → [metro-mvp][LOE:2][metro-it2]
(In reply to Axel Hecht [:Pike] from comment #12)
> * netError.dtd/appstrings.properties: Use the mobile version or the desktop
> version?

Yes -- in fact I wonder if we can get rid of our overridden netError.xhtml entirely (and just use the same page as desktop Firefox).

> * Are the strings we have there all used? I've come across bug 831099 in a
> quick search, but I'm not sure how extensive the research in that bug was?

In bug 831099 I took a quick glance through every DTD and .properties file and verified at least that there appears to be code using each string.  However, I might have missed some, or there might still be strings found only in unused or unneeded code.  For example, I just found and removed one in bug 835554.
(Assignee)

Comment 14

5 years ago
Created attachment 707808 [details] [diff] [review]
remove defines.inc

(In reply to Axel Hecht [:Pike] from comment #12)
> I guess we should also be able to share the defines.inc and -l10n.js file in
> http://hg.mozilla.org/projects/elm/file/4c9d12cd786e/browser/metro/locales/
> en-US/?

We're not even using defines.inc, I think that can just come out.

I'll also look at re-using l10n.js.
Attachment #707808 - Flags: review?(mbrubeck)
(Assignee)

Updated

5 years ago
Whiteboard: [metro-mvp][LOE:2][metro-it2] → [metro-mvp][LOE:2][metro-it2][leave-open]
(Assignee)

Comment 15

5 years ago
Created attachment 707814 [details] [diff] [review]
use fx l10s pref file
Attachment #707814 - Flags: review?(mbrubeck)
(Assignee)

Updated

5 years ago
Depends on: 836035
(Assignee)

Updated

5 years ago
Depends on: 836038
Comment on attachment 707808 [details] [diff] [review]
remove defines.inc

Seems okay to me, but I'm not really clear on what this file was for to begin with.  Is it used by langpacks?  Looking for a second opinion.
Attachment #707808 - Flags: review?(mbrubeck)
Attachment #707808 - Flags: review?(mark.finkle)
Attachment #707808 - Flags: review+
Attachment #707814 - Flags: review?(mbrubeck) → review+
Comment on attachment 707808 [details] [diff] [review]
remove defines.inc

(In reply to Matt Brubeck (:mbrubeck) from comment #16)
> Comment on attachment 707808 [details] [diff] [review]
> remove defines.inc
> 
> Seems okay to me, but I'm not really clear on what this file was for to
> begin with.  Is it used by langpacks?  Looking for a second opinion.

I think it was for langpacks and maybe even multi-locale builds. I see the MOZ_LANGPACK_CREATOR var is used in install.rdf, which is likely used by the langpack XPIs:
http://mxr.mozilla.org/projects-central/source/elm/browser/metro/locales/generic/install.rdf

I see that /browser already has defines.inc and install.rdf, and I assume those could be reused:
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/defines.inc

We could probably remove more from /browser/metro, but that can happen in other patches.
Attachment #707808 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 18

5 years ago
Yes this all langpack related. I think the install.rdf can come out too since our resources will be packed up in same xpi files used by the desktop, so both browsers will share the same langpack files. I've been experimenting with that in bug 802254.

Generally though at first we won't support langppacks in metro, so any resource that might confuse localizers needs to come out at first. We may bring some of these files back later.
(Assignee)

Comment 19

5 years ago
Created attachment 708100 [details] [diff] [review]
remove install.rdf
(Assignee)

Comment 20

5 years ago
I've filed bug 836266 on getting firefox langpacks working with metro. I'll leave it to Asa to triage that for v1.
(Assignee)

Comment 21

5 years ago
Created attachment 708151 [details] [diff] [review]
remove dead strings and code
Attachment #708151 - Flags: review?(mbrubeck)
Attachment #708151 - Flags: review?(mbrubeck) → review+
Depends on: 836421
(Assignee)

Updated

5 years ago
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 841058
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.