Closed Bug 1182722 Opened 9 years ago Closed 9 years ago

Migrate DevTools l10n under /devtools

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox45 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 45
Tracking Flags:
Tracking Status
firefox45 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

(Blocks 2 open bugs)

Details

Attachments

(9 files)

MozReview Request: Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau
40 bytes, text/x-review-board-request
ochameau
: review+
Details
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools client. r=glandium
40 bytes, text/x-review-board-request
glandium
: review+
Details
MozReview Request: Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau
40 bytes, text/x-review-board-request
ochameau
: review+
Details
MozReview Request: Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau
40 bytes, text/x-review-board-request
ochameau
: review+
Details
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium
40 bytes, text/x-review-board-request
glandium
: review+
Details
MozReview Request: Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau
40 bytes, text/x-review-board-request
ochameau
: review+
Details
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike
40 bytes, text/x-review-board-request
Pike
: review+
glandium
: review+
Details
Migration Script for l10n Repos
107 bytes, text/plain
Pike
: feedback+
Details
MozReview Request: Bug 1182722 - Clean up DevTools tests after l10n move. r=bgrins
40 bytes, text/x-review-board-request
bgrins
: review+
Details 
In bug 912121, we are planning to reorg DevTools code under a new top-level /devtools directory.

The migration plan for just code itself looks roughly like:

* browser/devtools -> devtools/client
* toolkit/devtools/server -> devtools/server
* toolkit/devtools -> devtools/shared

We need to plan for the l10n consequences of this.  In particular, we don't want to force all the translations to be redone.

The l10n files will need to move as well.  My current thinking is something like:

* browser/locales/en-US/chrome/browser/devtools -> devtools/client/locales/en-US
* toolkit/locales/en-US/chrome/global/devtools -> devtools/server/locales/en-US

The client / server division is important: only desktop Firefox ships the client, while everyone ships the server.
Pike, does my l10n migration proposal above seem reasonable?

How can we help l10n teams through the transition?  Write a script to move files?  Something else?
Flags: needinfo?(l10n)
For the folks working on directly mercurial, that'd be good, maybe even for those that don't.

We'll need to carefully look at things like l10n.ini etc to make sure we're picking up the right files in the right apps.

Talking to flod about this, we also wonder, when debugging against Android, do we show non-localized strings? Maybe we should test this against a French Android phone or so.
Flags: needinfo?(l10n)
Currently planning to move the l10n files *after* the larger code move.
Blocks: dt-contribute
No longer blocks: 912121
Depends on: 912121
Blocks: smdevtools
(In reply to Axel Hecht [:Pike] from comment #2)
> Talking to flod about this, we also wonder, when debugging against Android,
> do we show non-localized strings? Maybe we should test this against a French
> Android phone or so.

There are only a few strings on the server side currently, to show things like "Allow this connection?" on Android / B2G.  Actually, in many cases those products override the default strings DevTools code uses because they implement product-specific UI for these prompts.

Nearly all of the strings DevTools has are desktop Firefox only.
Slight adjustment to plan in comment 0:

Most of the strings from toolkit are used in both client and server, so they'll go to /devtools/shared, not /devtools/server.

* toolkit/locales/en-US/chrome/global/devtools -> devtools/shared/locales/en-US
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau
Attachment #8677621 - Flags: review?(poirot.alex)
Bug 1182722 - Update l10n packaging for DevTools client. r=glandium
Attachment #8677622 - Flags: review?(mh+mozilla)
Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau
Attachment #8677624 - Flags: review?(poirot.alex)
Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium
Attachment #8677625 - Flags: review?(mh+mozilla)
Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike
Attachment #8677627 - Flags: review?(mh+mozilla)
Attachment #8677627 - Flags: review?(l10n)
I ran a test migration locally using the script with the fr locale.  I built a langpack and was able to see all translated DevTools strings as expected.

Assuming it looks good, I'll notify the l10n list about this move before landing.
Attachment #8677632 - Flags: feedback?(l10n)
Attachment #8677632 - Attachment mime type: application/x-sh → text/plain
Attachment #8677622 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8677622 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools client. r=glandium

https://reviewboard.mozilla.org/r/22979/#review20535

::: devtools/client/locales/jar.mn:8
(Diff revision 1)
> +    locale/@AB_CD@/devtools/ (%*)

I'm sure gps would have something to say about the use of wildcards, but that can be adjusted afterwards (as in, in a followup) if necessary.
Comment on attachment 8677625 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium

https://reviewboard.mozilla.org/r/22985/#review20537
Attachment #8677625 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

https://reviewboard.mozilla.org/r/22989/#review20539

Looks like all this series should be folded when landing.
Attachment #8677627 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8677621 [details]
MozReview Request: Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau

Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau
Comment on attachment 8677622 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools client. r=glandium

Bug 1182722 - Update l10n packaging for DevTools client. r=glandium
Comment on attachment 8677623 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau

Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau
Comment on attachment 8677624 [details]
MozReview Request: Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau

Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau
Comment on attachment 8677625 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium

Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium
Comment on attachment 8677626 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau

Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

:glandium, you may want to take another look at the build changes.  After folding devtools back into /toolkit in bug 1217687, I thought it seemed best for toolkit to also control the devtools/shared l10n files, so that changed the structure slightly.
Attachment #8677627 - Flags: review+ → review?(mh+mozilla)
There is some magic mapping that I would like to avoid if possible.
Both /devtools/client/locales and /devtools/shared/locales maps to chrome://devtools/locale/
That would prevent mapping locales to a local m-c checkout dynamically.
Do you think we could use ressource:// uri for locales?
I really appreciate our new resource:// uris as it is a straightforward 1-1 mapping,
strip the "resource:/" from the url and you get the relative path from mozilla-central top-level directory!
Otherwise, if that's too complex, we could use the same trick than for existing chrome:// uris by parsing jar.mn files and doing one override per l10n file.
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

https://reviewboard.mozilla.org/r/22989/#review20897

I think this should work, r=me with the nits below.

::: browser/locales/filter.py:14
(Diff revision 2)
> +                 "devtools/client", "devtools/shared"):

I'd prefer to have these ordered semantically, I think between the toolkit and the browser portion is a nice fit for this line.

::: mobile/android/locales/filter.py:16
(Diff revision 2)
> +                 "devtools/shared"):

Same here, move this up?
Attachment #8677627 - Flags: review?(l10n) → review+
(In reply to Alexandre Poirot [:ochameau] from comment #27)
> There is some magic mapping that I would like to avoid if possible.
> Both /devtools/client/locales and /devtools/shared/locales maps to
> chrome://devtools/locale/
> That would prevent mapping locales to a local m-c checkout dynamically.
> Do you think we could use ressource:// uri for locales?
> I really appreciate our new resource:// uris as it is a straightforward 1-1
> mapping,
> strip the "resource:/" from the url and you get the relative path from
> mozilla-central top-level directory!
> Otherwise, if that's too complex, we could use the same trick than for
> existing chrome:// uris by parsing jar.mn files and doing one override per
> l10n file.

resource: urls won't work, as they're not going through the chrome registry and thus don't get locale selection. This is in particular affecting multi-locale builds on linux. Also probably breaks language packs.

The chrome package can only be a single thing, but something like chrome://devtools-shared/locale would work.
Comment on attachment 8677632 [details]
Migration Script for l10n Repos

Thanks. I wonder how to land this in the smoothest way possible, too.

Maybe coordinate with sheriffs so that we don't get in the middle of back-outs and stuff? Maybe something like "first thing after merge of fx-team, and merge again if green" or so?
Attachment #8677632 - Flags: feedback?(l10n) → feedback+
I imagine we can't have chrome://devtools/client/locale/, chrome://devtools/shared/locale/ as well.
The good thing with chrome://devtools/content/ is that it maps only to /devtools/client,
so it is not that magical. We still have to figure out that it maps there, but it is a simple mapping.
Here, it is more complex, and I don't think we can map that easily without involving a build system that will collapse the two into one.
I imagine it also open way to have file collisions between client and shared?
chrome://devtools-shared/locale and chrome://devtools-client/locale or chrome://devtools/locale would work but looks like a workaround.
(In reply to Alexandre Poirot [:ochameau] from comment #27)
> There is some magic mapping that I would like to avoid if possible.
> Both /devtools/client/locales and /devtools/shared/locales maps to
> chrome://devtools/locale/
> That would prevent mapping locales to a local m-c checkout dynamically.
> Do you think we could use ressource:// uri for locales?
> I really appreciate our new resource:// uris as it is a straightforward 1-1
> mapping,
> strip the "resource:/" from the url and you get the relative path from
> mozilla-central top-level directory!
> Otherwise, if that's too complex, we could use the same trick than for
> existing chrome:// uris by parsing jar.mn files and doing one override per
> l10n file.

As Pike says, I believe we have to use chrome:// here, until we cook up some entirely different l10n mechanism, like Gaia for example.

I agree it's not great, but the build system handling of l10n is quite complex, so I think it's best to follow existing examples closely, so we (mostly me) don't pull our hair out too much over this.

Adding a new l10n file is quite a rare operation, roughly once per tool currently.  I think I am willing to live with slightly strange paths for this case in order to at least have things under /devtools without too much headache.  At least the files are much closer to our tools in the source tree, which is a big improvement.

We could also try the same thing I used for chrome:// themes, where I used the exact path, but replaced the second word with "skin" as required by the chrome:// protocol.  In this case, it would look like:

  chrome://devtools/locale/locales/debugger.dtd

instead of the current:

  chrome://devtools/locale/debugger.dtd

since "locale" is required to be path segment 2.  I am not sure either one is really much better than the other...  I don't really like either, but we do need something like these.

For our reload feature, I think we can make do with having one override per file.

(In reply to Alexandre Poirot [:ochameau] from comment #31)
> I imagine we can't have chrome://devtools/client/locale/,
> chrome://devtools/shared/locale/ as well.

No, unfortunately we can't do that.  This same issue came up earlier with chrome:// content and themes.  If that were possible, I would have already done it.  The issue is that the chrome:// protocol handler requires that path segment 2 is one of the words "content", "skin", or "locale".

> The good thing with chrome://devtools/content/ is that it maps only to
> /devtools/client,
> so it is not that magical. We still have to figure out that it maps there,
> but it is a simple mapping.
> Here, it is more complex, and I don't think we can map that easily without
> involving a build system that will collapse the two into one.
> I imagine it also open way to have file collisions between client and shared?

I suppose it is possible to have collisions, but I don't think that will be an issue in practice.  The rate of new files, especially on the shared side, is extremely low.

By the time we hit such a problem, I hope we are already using some other l10n process anyway because have dropped chrome:// altogether! :)

> chrome://devtools-shared/locale and chrome://devtools-client/locale or
> chrome://devtools/locale would work but looks like a workaround.

I am not very excited about using devtools-shared and devtools-client just for l10n files.  Maybe if we were also doing so for the other DevTools files, but so far we've managed to avoid it for the rest of DevTools files (the non-l10n files).  I think "chrome://devtools/" feels nicer than these alternatives, but that's my subjective opinion only.

If you have suggestions about other adjustments to the path the comes **after** chrome://devtools/locale, we can change that pretty easily.

I think it's probably not worth worrying too much here.  The real clean up will happen when we move tools away from chrome:// and need to do something else anyway.
Depends on: 1219035
Depends on: 1219039
Attachment #8677627 - Flags: review?(mh+mozilla)
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

https://reviewboard.mozilla.org/r/22989/#review20945

::: browser/locales/Makefile.in:130
(Diff revision 2)
> -	@$(MAKE) -C ../../toolkit/locales libs-$*
> +	@$(MAKE) -C ../../toolkit/locales libs-$* XPI_ROOT_APPID=$(XPI_ROOT_APPID)

Why do you need XPI_ROOT_APPID?
https://reviewboard.mozilla.org/r/22989/#review20945

> Why do you need XPI_ROOT_APPID?

XPI_ROOT_APPID is needed if DIST_SUBDIR is used, according to rules.mk[1].

We currently traverse down directories in the following manner:

1. /browser/locales
2. /toolkit/locales
3. /devtools/shared/locales

Because DIST_SUBDIR is in effect when building /devtools files and DIST_SUBDIR requires XPI_ROOT_APPID, we need to pass it along to /toolkit/locales, which then also passes it to /devtools/shared/locales.

I know nothing about XPI_ROOT_APPID, I am just complying with the error message.

[1]: https://dxr.mozilla.org/mozilla-central/source/config/rules.mk#1252
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

https://reviewboard.mozilla.org/r/22989/#review20961

Ok, makes sense.
Attachment #8677627 - Flags: review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #32)
> We could also try the same thing I used for chrome:// themes, where I used
> the exact path, but replaced the second word with "skin" as required by the
> chrome:// protocol.  In this case, it would look like:
> 
>   chrome://devtools/locale/locales/debugger.dtd
> 
> instead of the current:
> 
>   chrome://devtools/locale/debugger.dtd
> 
> since "locale" is required to be path segment 2.  I am not sure either one
> is really much better than the other...  I don't really like either, but we
> do need something like these.
> 
> For our reload feature, I think we can make do with having one override per
> file.

I'm currently having an addon that doesn't require any override, even for chrome content/skin resources.
But I would have to keep overrides for l10n if we keep this merge between the two /devtools/client/locale and /devtools/shared/locale. The addon is really simple stupid:
  content devtools mozilla-central/devtools/client/
  skin devtools classic/1.0 mozilla-central/devtools/client/

The extra "themes" in the URI may help to better match the full relative path from mozilla-central,
but it's still hawkward to have to say "content" is equivalent to "client".
And here it forces to map chrome://devtools/skin/ to /devtools/client/, which is a bit weird but not a big deal. I would say there isn't much value in adding the "locales" or "themes" in the URI.

> (In reply to Alexandre Poirot [:ochameau] from comment #31)
> > chrome://devtools-shared/locale and chrome://devtools-client/locale or
> > chrome://devtools/locale would work but looks like a workaround.
> 
> I am not very excited about using devtools-shared and devtools-client just
> for l10n files.  

I'm not excisted about devtools-shared and devtools-client either.
But having chrome://devtools and chrome://devtools-shared may be a good option.
To get back to the addon that allows to easily hack on /devtools codebase without restarting firefox,
I try to imagine how to make it work for the locales.
With the current proposal I could do:
  locale devtools mozilla-central/devtools/client/locales
And then do manual override for each locale files from devtools/shared/locales.
With having chrome://devtools and chrome://devtools-shared I could do:
  locale devtools mozilla-central/devtools/client/locales
  locale devtools-shared mozilla-central/devtools/shared/locales

> Maybe if we were also doing so for the other DevTools
> files, but so far we've managed to avoid it for the rest of DevTools files
> (the non-l10n files).  I think "chrome://devtools/" feels nicer than these
> alternatives, but that's my subjective opinion only.

The thing is that we are not doing such merge between two folders for other devtools files.
chrome://devtools/ only maps to /devtools/client/
chrome://devtools/content/ and chrome://devtools/skin/ both maps to /devtools/client.

/devtools/shared and /devtools/server are both only using resource://devtools/.

> If you have suggestions about other adjustments to the path the comes
> **after** chrome://devtools/locale, we can change that pretty easily.

I don't think it will help much.
But what about keeping chrome://devtools/locale/ to map to /devtools/client/locales (or /devtools/client/ and add /locales/ in all URI as for themes) and have a second special mapping for shared locales: chrome://devtools-shared/locale to map to /devtools/shared/locales?
This won't be landing until next cycle (45) due to try issues I still need to resolve and also the l10n team is quite busy handling the upcoming early merge.
I think I've come around to Alex's ideas.  I'll try devtools and devtools-shared.
https://reviewboard.mozilla.org/r/22989/#review20897

> I'd prefer to have these ordered semantically, I think between the toolkit and the browser portion is a nice fit for this line.

Okay, I've moved them all up like you suggest.
Comment on attachment 8677621 [details]
MozReview Request: Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau

Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau
Comment on attachment 8677622 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools client. r=glandium

Bug 1182722 - Update l10n packaging for DevTools client. r=glandium
Comment on attachment 8677623 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau

Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau
Comment on attachment 8677624 [details]
MozReview Request: Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau

Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau
Comment on attachment 8677625 [details]
MozReview Request: Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium

Bug 1182722 - Update l10n packaging for DevTools shared. r=glandium
Comment on attachment 8677626 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau

Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau
Comment on attachment 8677627 [details]
MozReview Request: Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike

Bug 1182722 - Add DevTools folders to l10n machinery. r=glandium,Pike
Comment on attachment 8680922 [details]
MozReview Request: Bug 1182722 - Clean up DevTools tests after l10n move. r=bgrins

https://reviewboard.mozilla.org/r/23689/#review21209
Attachment #8680922 - Flags: review?(bgrinstead) → review+
Comment on attachment 8677621 [details]
MozReview Request: Bug 1182722 - Move DevTools client en-US l10n files. r=ochameau

https://reviewboard.mozilla.org/r/22977/#review21355
Attachment #8677621 - Flags: review?(poirot.alex) → review+
Comment on attachment 8677623 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools client l10n URLs. r=ochameau

https://reviewboard.mozilla.org/r/22981/#review21357

Be sure to checkout very last changes before landing.
Attachment #8677623 - Flags: review?(poirot.alex) → review+
Comment on attachment 8677624 [details]
MozReview Request: Bug 1182722 - Move DevTools shared en-US l10n files. r=ochameau

https://reviewboard.mozilla.org/r/22983/#review21359

Thanks doing that at the end!
Attachment #8677624 - Flags: review?(poirot.alex) → review+
Comment on attachment 8677626 [details]
MozReview Request: Bug 1182722 - Rewrite DevTools shared l10n URLs. r=ochameau

https://reviewboard.mozilla.org/r/22987/#review21361

Note that I'm about to land bug 1216590 (last app-manager cleanup) that removes files and conflict with one of this serie.
Attachment #8677626 - Flags: review?(poirot.alex) → review+
I have received IRC approval from Pike to proceed with landing.
https://hg.mozilla.org/mozilla-central/rev/a907d1913710
https://hg.mozilla.org/mozilla-central/rev/eb607c348695
https://hg.mozilla.org/mozilla-central/rev/ff8ebe5251ec
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Blocks: 1223338
url:        https://hg.mozilla.org/l10n-central/ru/rev/5e76bbe3823dfb807087182c31eda621a0ef7e04
changeset:  5e76bbe3823dfb807087182c31eda621a0ef7e04
user:       Axel Hecht <axel@pike.org>
date:       Mon Dec 14 11:59:02 2015 -0800
description:
bug 1182722, move devtools
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: