Closed
Bug 687851
Opened 13 years ago
Closed 13 years ago
Create a locales directory in the devtools module, move all devtools l10n strings to it
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 10
People
(Reporter: ddahl, Assigned: jwalker)
References
Details
(Whiteboard: [fixed-in-fx-team])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Most of our strings will be optional, and we would like to be able to review them ourselves
Reporter | ||
Updated•13 years ago
|
Summary: Create a locales directory in the devtools module, move all devtools l10n stings to it → Create a locales directory in the devtools module, move all devtools l10n strings to it
Comment 1•13 years ago
|
||
Please don't put files into browser/devtools/locales/en-US or something. Just use a folder within the browser/locales/en-US/ hierarchy?
In the l10n sources, the /locales/en-US/ part is dropped, so if you add stuff within browser/something, the en-US location becomes ambiguous from an l10n point of view.
I'd suggest to go for browser/locales/en-US/chrome/devtools or ...chrome/browser/devtools.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → jwalker
Assignee | ||
Comment 2•13 years ago
|
||
In browser/devtools there is a hierachy for each tool. I suggest we replicate that inside browser/locales/en-US/chrome/devtools.
Comment 3•13 years ago
|
||
(In reply to Joe Walker from comment #2)
> In browser/devtools there is a hierachy for each tool. I suggest we
> replicate that inside browser/locales/en-US/chrome/devtools.
We generally have a dtd and .properties file for each dev tool. Not sure there's value in having subfolders for each dev tool. Wouldn't it be sufficient to just put our dev tools dtd and properties files inside a single browser/locales/en-US/devtools folder?
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to Mihai Sucan [:msucan] from comment #3)
> (In reply to Joe Walker from comment #2)
> > In browser/devtools there is a hierachy for each tool. I suggest we
> > replicate that inside browser/locales/en-US/chrome/devtools.
>
> We generally have a dtd and .properties file for each dev tool. Not sure
> there's value in having subfolders for each dev tool. Wouldn't it be
> sufficient to just put our dev tools dtd and properties files inside a
> single browser/locales/en-US/devtools folder?
webconsole has several already, but I'm not fussed. IMHO it would slow navigation down if we got more than 20/30 files in the one directory.
Comment 5•13 years ago
|
||
Sure, that works then. Subfolders it is.
Assignee | ||
Comment 6•13 years ago
|
||
Axel, in bug 656666 comment 14, you pointed out the cost in moving a strings file. Am I right in thinking that either we're doing this soon enough, or that the pain is worth it - i.e. that we should carry on ...
Assignee | ||
Comment 7•13 years ago
|
||
We're currently working on a patch to use:
/browser/locales/en-US/chrome/browser/devtools/PROJECT
Where PROJECT is one of highlighter, scratchpad, shared, sourceeditor, styleinspector, webconsole, etc.
I think everyone is happy with this. Please shout ASAP if not.
Comment 8•13 years ago
|
||
The extra nesting of a directory-per-tool seems overkill. Won't most of them have only one or two files? Directories with 20-30 files (are there even that many now?) isn't particularly troublesome.
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #8)
> The extra nesting of a directory-per-tool seems overkill. Won't most of them
> have only one or two files? Directories with 20-30 files (are there even
> that many now?) isn't particularly troublesome.
It's hard to know how many each will have - Perhaps 2 would be the norm, but it depends on how we split our tools up. I would hope that we grow our tools rather than adding many new ones, which indicates more than 2 files. webconsole is already at least 4 (probably more but it's hard to tell from a quick scan)
Whether 30 files is troublesome depends on your definition of troublesome I guess. Obviously is no problems with directory size or anything, but it is possibly slower to find what you're looking for.
I'm not particularly fussed however.
Comment 10•13 years ago
|
||
I prefer less nesting as a rule. ~30 files doesn't seem like too many to me, really.
Assignee | ||
Comment 11•13 years ago
|
||
OK, Ok, ok. Clearly outvoted. We'll go flatter.
Joe.
Assignee | ||
Comment 12•13 years ago
|
||
This patch moves all devtools strings to /browser/locales/en-US/chome/browser/devtools
It includes change to /mobile/chrome/content/content.js since it refers to headsUpDisplay.properties
Attachment #566522 -
Flags: review?(rcampbell)
Attachment #566522 -
Flags: review?(mark.finkle)
Attachment #566522 -
Flags: review?(l10n)
Comment 13•13 years ago
|
||
Comment on attachment 566522 [details] [diff] [review]
upload 1
Firefox Mobile can't access anything in /browser, since that code is not built for Firefox Mobile. We depend on toolkit code for "shared" resources.
Since we are only using a single string, let's just move the string to a /mobile properties file.
Attachment #566522 -
Flags: review?(mark.finkle) → review-
Comment 14•13 years ago
|
||
Comment on attachment 566522 [details] [diff] [review]
upload 1
I really wish we had done this earlier, but in the long term, it's going to pay off.
We'll need noise around this, and mentors to help out, to deal with the fallout on the l10n community.
Attachment #566522 -
Flags: review?(l10n) → review+
Assignee | ||
Comment 15•13 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #13)
> Comment on attachment 566522 [details] [diff] [review] [diff] [details] [review]
> upload 1
>
> Firefox Mobile can't access anything in /browser, since that code is not
> built for Firefox Mobile. We depend on toolkit code for "shared" resources.
>
> Since we are only using a single string, let's just move the string to a
> /mobile properties file.
Can you recommend one to use? There are no obvious candidates.
My best guess is to leave a stripped out headsUpDisplay.properties in place containing just the 2 strings in question.
Comment 16•13 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #13)
> Comment on attachment 566522 [details] [diff] [review] [diff] [details] [review]
> upload 1
>
> Firefox Mobile can't access anything in /browser, since that code is not
> built for Firefox Mobile. We depend on toolkit code for "shared" resources.
>
> Since we are only using a single string, let's just move the string to a
> /mobile properties file.
I'd recommend moving the string you need to /mobile rather than blocking this bug with an r-.
Comment 17•13 years ago
|
||
Comment on attachment 566522 [details] [diff] [review]
upload 1
looks good.
Attachment #566522 -
Flags: review?(rcampbell) → review+
Comment 18•13 years ago
|
||
PS, Mark, can you file a bug on your string migration and mark that is blocking this bug? We won't land until your move's complete. Hoping to get this landed by next week at the latest.
Updated•13 years ago
|
Status: NEW → ASSIGNED
Updated•13 years ago
|
Attachment #566522 -
Flags: feedback+
Assignee | ||
Comment 19•13 years ago
|
||
Mark: this implements my idea from comment 15 to keep the properties file in place stripped of the properties that you're not using.
I've r?ed Axel again because this changes the proposition.
We're keen to get one of these landed soon, but don't really care which way we do it.
Thanks.
Attachment #568015 -
Flags: review?(mark.finkle)
Attachment #568015 -
Flags: review?(l10n)
Comment 20•13 years ago
|
||
I'm a bit lost, I'm afraid. What's this review request about in detail?
Assignee | ||
Comment 21•13 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #20)
> I'm a bit lost, I'm afraid. What's this review request about in detail?
It's about moving all devtools l10n strings to the same place. You've r+ed it once before (see comment 14). That patch moved all the files to the 'right' place, however some time ago mobile began to use devtools strings, so this patch would have broken mobile.
There were 2 options:
1. in a separate bug, mobile creates their own new strings file
2. we copy rather than move, and then strip out all but the 2 strings mobile are using from the original
This is a busy week for mobile, so in an attempt to get things moving, I did 2.
I r?ed you because this changes the way we do things.
Comment 22•13 years ago
|
||
Comment on attachment 568015 [details] [diff] [review]
version 2
OK. This will keep Fennec working. Thanks!
Attachment #568015 -
Flags: review?(mark.finkle) → review+
Comment 23•13 years ago
|
||
Comment on attachment 568015 [details] [diff] [review]
version 2
Looks good to me.
When we communicate about this, we should have a script that does the file moves/copies for an l10n repo in hand, too.
The change in this patch is basically that we just want to copy headsUpDisplay.properties for l10n, too.
We won't need to bother about removing the obsolete strings, that's something the localizers can probably safely do themselves.
Attachment #568015 -
Flags: review?(l10n) → review+
Comment 24•13 years ago
|
||
Comment on attachment 568015 [details] [diff] [review]
version 2
>copy from toolkit/locales/en-US/chrome/global/headsUpDisplay.properties
>copy to browser/locales/en-US/chrome/browser/devtools/headsUpDisplay.properties
Can you rename this file to webconsole.properties while you're at it?
Comment 25•13 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #24)
> Comment on attachment 568015 [details] [diff] [review] [diff] [details] [review]
> version 2
>
> >copy from toolkit/locales/en-US/chrome/global/headsUpDisplay.properties
> >copy to browser/locales/en-US/chrome/browser/devtools/headsUpDisplay.properties
>
> Can you rename this file to webconsole.properties while you're at it?
good idea.
Assignee | ||
Comment 26•13 years ago
|
||
Attachment #566522 -
Attachment is obsolete: true
Attachment #568015 -
Attachment is obsolete: true
Assignee | ||
Comment 27•13 years ago
|
||
I pushed to try just now, but there appears to be some bustage there.
Whiteboard: [land-in-fx-team]
Comment 28•13 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5d250b1cf465
hopefully the bustage mentioned in Comment 27 is of the "try server is busted" variety.
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 29•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Target Milestone: --- → Firefox 10
Comment 30•13 years ago
|
||
I'm trying to fix my locale after this (painful), but there's something I don't understand.
inspector.properties was last seen here
http://hg.mozilla.org/mozilla-central/file/cd6ed3106521/browser/locales/en-US/chrome/browser/inspector.properties
When moved to /devtools a new string appear (breadcrumbs.siblings)
http://hg.mozilla.org/mozilla-central/file/e43a54b52b06/browser/locales/en-US/chrome/browser/devtools/inspector.properties#l10
I'm trying to understand in which bug this string was introduced (hg logs don't help).
Comment 31•13 years ago
|
||
Another thing (nitpicking): webconsole.properties vs webConsole.dtd
Comment 32•13 years ago
|
||
flod, breadcrumbs.siblings landed with bug 672006. They ended up in the same changeset together so I can see why that'd be confusing.
Please file bugs if you find anything odd, annoying or painful! thanks.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•