769677 - (metro-l10n) [Tracking] Review localizability of metro Firefox

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Axel Hecht [:Pike]]

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.

Comment 1

[Axel Hecht [:Pike]]

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.

Comment 2

[Jim Mathies [:jimm]]

(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.

Comment 3

[Robert Strong (they/them - no direct email)]

(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.

Comment 4

[Axel Hecht [:Pike]]

We should also package the same region.properties for search ordering etc.

Comment 5

[Justin Dolske [:Dolske]]

(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.

Comment 6

[Axel Hecht [:Pike]]

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.

Comment 7

[Jim Mathies [:jimm]]

(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.

Comment 8

[Jim Mathies [:jimm]]

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.

Comment 9

[Jim Mathies [:jimm]]

https://hg.mozilla.org/projects/elm/rev/b5c0a22549e6

Comment 10

[Jim Mathies [:jimm]]

(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.

Comment 11

[Matt Brubeck (:mbrubeck)]

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.

Comment 12

[Axel Hecht [:Pike]]

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/?

Comment 13

[Matt Brubeck (:mbrubeck)]

(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.

Comment 14

[Jim Mathies [:jimm]]

Attachment: 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.

Comment 15

[Jim Mathies [:jimm]]

Attachment: use fx l10s pref file

Comment 16

[Matt Brubeck (:mbrubeck)]

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.

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 18

[Jim Mathies [:jimm]]

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.

Comment 19

[Jim Mathies [:jimm]]

Attachment: remove install.rdf

Comment 20

[Jim Mathies [:jimm]]

I've filed bug 836266 on getting firefox langpacks working with metro. I'll leave it to Asa to triage that for v1.

Comment 21

[Jim Mathies [:jimm]]

Attachment: remove dead strings and code

Comment 22

[Jim Mathies [:jimm]]

https://hg.mozilla.org/projects/elm/rev/101f09ac1ceb
https://hg.mozilla.org/projects/elm/rev/8415679d966d
https://hg.mozilla.org/projects/elm/rev/ec6193b8e231
https://hg.mozilla.org/projects/elm/rev/4db2da670ea0