Closed
Bug 1294220
Opened 8 years ago
Closed 8 years ago
devtools.html localization: remove dependencies on Services.strings in l10n.js
Categories
(DevTools :: General, defect, P1)
DevTools
General
Tracking
(firefox51 fixed)
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
(Whiteboard: [devtools-html])
Attachments
(6 files, 2 obsolete files)
58 bytes,
text/x-review-board-request
|
bgrins
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
bgrins
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
bgrins
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
tromey
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
tromey
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
tromey
:
review+
|
Details |
For devtools.html localization, we need to be able to synchronously load properties files.
Assignee | ||
Comment 1•8 years ago
|
||
Probably needs to wait for Bug 1248830, to be discussed in triage.
Depends on: 1248830
Whiteboard: [devtools-html] [triage]
Updated•8 years ago
|
Blocks: devtools-html-phase2, 1290947
Flags: qe-verify?
Updated•8 years ago
|
Flags: qe-verify? → qe-verify-
Priority: -- → P2
Whiteboard: [devtools-html] [triage] → [devtools-html]
Assignee | ||
Comment 2•8 years ago
|
||
Repurposing this bug. It seems that purely loading raw files is already taken care of by the loader. See Bug 1290947 comment 8 (In reply to Tom Tromey :tromey from comment #8) > > 2.1: create a loader to read raw/properties files > > You can use require("raw!devtools/path/to/whatever") to get a file's > contents. > E.g., > https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/theme. > js#15 Consequently, we do not need to do anything specific for properties files here. The next step is to come up with a new version of shared/l10n.js that does not rely on Services.string.
Summary: devtools.html localization: investigate loading of properties files → devtools.html localization: remove dependencies on Services.strings in l10n.js
Assignee | ||
Comment 3•8 years ago
|
||
My initial approach here is to use libraries in order to: - parse the properties files (using a modified version of https://github.com/gagle/node-properties) - format the strings relying on %S and similar (using a modified version of https://github.com/alexei/sprintf.js) This seems to work pretty well on the few modules I tested so far. I tried to look for similar helpers in our current codebase but could not find any. ni? to Tom and Brian in case I missed an existing helper I should use instead? Another question: should such external libraries go in devtools/client/shared/vendor (knowing they both need to be modified before using them)?
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Flags: needinfo?(ttromey)
Flags: needinfo?(bgrinstead)
Priority: P2 → P1
Comment hidden (mozreview-request) |
Updated•8 years ago
|
Iteration: --- → 51.2 - Aug 29
Comment 5•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #3) > My initial approach here is to use libraries in order to: > - parse the properties files (using a modified version of > https://github.com/gagle/node-properties) > - format the strings relying on %S and similar (using a modified version of > https://github.com/alexei/sprintf.js) I think importing external code needs some kind of license review. I don't know how to do this, but I think :jryans does. Don't let this dissuade you from using external modules, supposedly the process isn't difficult. > I tried to look for similar helpers in our current codebase but could not > find any. > ni? to Tom and Brian in case I missed an existing helper I should use > instead? I'm not aware of any (but I'm not sure I would be). > Another question: should such external libraries go in > devtools/client/shared/vendor (knowing they both need to be modified before > using them)? I think that's quite reasonable. It would be good to have a large comment or README explaining where the files came from and how they were modified (perhaps with an upstream commit hash so we could reliably diff against the baseline revision).
Flags: needinfo?(ttromey)
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8782397 [details] Bug 1294220 - part1: Move ellipsis character to a localized string in properties file; https://reviewboard.mozilla.org/r/72582/#review70278 I made a few notes. ::: devtools/client/shared/l10n.js:20 (Diff revision 1) > * The desired string bundle's name. > */ > function LocalizationHelper(stringBundleName) { > - loader.lazyGetter(this, "stringBundle", () => > - Services.strings.createBundle(stringBundleName)); > + this.stringBundleName = stringBundleName; > + loader.lazyGetter(this, "properties", () => { > + let file = require("raw!" + this.stringBundleName); This seems reasonable. The dynamic lookup here means we'll have to be a bit more careful in our webpack config. Also I wonder what URLs are being passed in here -- if chrome: URLs then those won't work in content... ::: devtools/client/shared/l10n.js:25 (Diff revision 1) > + let file = require("raw!" + this.stringBundleName); > + return parsePropertiesFile(file); > + }); > + > loader.lazyGetter(this, "ellipsis", () => > - Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString) > + Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data); We want to avoid Ci and require("chrome") in content. It's fine I think to defer that to another patch but it's worth considering now what that will look like. Also, we already have a content-only Services shim, so if it's convenient we could move some of the logic there. ::: devtools/client/shared/vendor/moz.build:10 (Diff revision 1) > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > modules = [] > modules += [ > 'immutable.js', > - 'jsol.js' > + 'jsol.js', > + 'parse.js' I try to always use a trailing "," in spots like this so that a patch doesn't have to also touch the previous line. This makes history navigation a bit easier sometimes.
Assignee | ||
Comment 7•8 years ago
|
||
Thanks for having a look ! (In reply to Tom Tromey :tromey from comment #6) > Comment on attachment 8782397 [details] > Bug 1294220 - devtools shared l10n helper: remove dependency on > Services.string > > https://reviewboard.mozilla.org/r/72582/#review70278 > > I made a few notes. > > ::: devtools/client/shared/l10n.js:20 > (Diff revision 1) > > * The desired string bundle's name. > > */ > > function LocalizationHelper(stringBundleName) { > > - loader.lazyGetter(this, "stringBundle", () => > > - Services.strings.createBundle(stringBundleName)); > > + this.stringBundleName = stringBundleName; > > + loader.lazyGetter(this, "properties", () => { > > + let file = require("raw!" + this.stringBundleName); > > This seems reasonable. The dynamic lookup here means we'll have to be a bit > more careful in our webpack config. Also I wonder what URLs are being > passed in here -- if chrome: URLs then those won't work in content... > I plan to make another patch to make sure we don't pass chrome:// urls here. I started thinking about the impact that localization can have on our webpack strategy, I need to spend more time on this. (I don't think there's a way to avoid the dynamic lookup here?) > ::: devtools/client/shared/l10n.js:25 > (Diff revision 1) > > + let file = require("raw!" + this.stringBundleName); > > + return parsePropertiesFile(file); > > + }); > > + > > loader.lazyGetter(this, "ellipsis", () => > > - Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString) > > + Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data); > > We want to avoid Ci and require("chrome") in content. It's fine I think to > defer that to another patch but it's worth considering now what that will > look like. > > Also, we already have a content-only Services shim, so if it's convenient we > could move some of the logic there. > I would like to also fix this here. I think I can modify the preferences actor to return complex values and use this here in order to get the ellipsis. This way l10n.js will be free of any chrome privileged API. > ::: devtools/client/shared/vendor/moz.build:10 > (Diff revision 1) > > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > modules = [] > > modules += [ > > 'immutable.js', > > - 'jsol.js' > > + 'jsol.js', > > + 'parse.js' > > I try to always use a trailing "," in spots like this so that a patch > doesn't have to also touch the previous line. This makes history navigation > a bit easier sometimes. Good point!
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Tom Tromey :tromey from comment #5) > (In reply to Julian Descottes [:jdescottes] from comment #3) > > My initial approach here is to use libraries in order to: > > - parse the properties files (using a modified version of > > https://github.com/gagle/node-properties) > > - format the strings relying on %S and similar (using a modified version of > > https://github.com/alexei/sprintf.js) > > I think importing external code needs some kind of license review. > I don't know how to do this, but I think :jryans does. > Don't let this dissuade you from using external modules, supposedly the > process isn't difficult. > jryans: do you know the process to include external libraries such as the ones mentioned above?
Flags: needinfo?(jryans)
Comment 9•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #7) > > ::: devtools/client/shared/l10n.js:25 > > (Diff revision 1) > > > + let file = require("raw!" + this.stringBundleName); > > > + return parsePropertiesFile(file); > > > + }); > > > + > > > loader.lazyGetter(this, "ellipsis", () => > > > - Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString) > > > + Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data); > > > > We want to avoid Ci and require("chrome") in content. It's fine I think to > > defer that to another patch but it's worth considering now what that will > > look like. > > > > Also, we already have a content-only Services shim, so if it's convenient we > > could move some of the logic there. > > > > I would like to also fix this here. I think I can modify the preferences > actor to return complex values and use this here in order to get the > ellipsis. This way l10n.js will be free of any chrome privileged API. > I don't think we want the preferences actor to be involved here - that would be only if we wanted to fetch prefs on the target being debugged. This also reminds me of an issue here which is we aren't directly setting intl.ellipsis value in devtools.js so when we load the file in content the pref probably won't exist. We could move that out into another bug, though.
Comment 10•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #8) > (In reply to Tom Tromey :tromey from comment #5) > > (In reply to Julian Descottes [:jdescottes] from comment #3) > > > My initial approach here is to use libraries in order to: > > > - parse the properties files (using a modified version of > > > https://github.com/gagle/node-properties) > > > - format the strings relying on %S and similar (using a modified version of > > > https://github.com/alexei/sprintf.js) > > > > I think importing external code needs some kind of license review. > > I don't know how to do this, but I think :jryans does. > > Don't let this dissuade you from using external modules, supposedly the > > process isn't difficult. > > > > jryans: do you know the process to include external libraries such as the > ones mentioned above? Tangential to the license review: for things like codemirror, there's an update process that includes applying a patch to produce a modified version of the library: https://dxr.mozilla.org/mozilla-central/source/devtools/client/sourceeditor/codemirror/README#47. I'm not sure what modifications were needed for these libs, but we should document the update process - including the current version number, link to the lib, license, and any patches needed for updates
(In reply to Julian Descottes [:jdescottes] from comment #8) > jryans: do you know the process to include external libraries such as the > ones mentioned above? For license review, look over the license policy page[1]. Basically, file a licensing team bug[2] to get a clear answer from them on how to proceed. Depending on the license, you may need to add the license to the about:license page. [1]: https://www.mozilla.org/en-US/MPL/license-policy/ [2]: https://bugzilla.mozilla.org/enter_bug.cgi?bug_severity=normal&bug_status=NEW&component=Licensing&op_sys=All&product=mozilla.org&rep_platform=All
Flags: needinfo?(jryans)
Comment 12•8 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #9) > I don't think we want the preferences actor to be involved here - that would > be only if we wanted to fetch prefs on the target being debugged. We definitely don't want the prefs actor. We have a prefs implementation in our content Services shim. > This also reminds me of an issue here which is we aren't directly setting > intl.ellipsis value in devtools.js so when we load the file in content the > pref probably won't exist. We could move that out into another bug, though. :bgrins noted in the other bug that we could put the intl.ellipsis value into a properties file instead and just treat it like other localized text.
Comment 13•8 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #9) > This also reminds me of an issue here which is we aren't directly setting > intl.ellipsis value in devtools.js so when we load the file in content the > pref probably won't exist. We could move that out into another bug, though. We have to read at least part of all.js anyway, because not all devtools prefs are in devtools.js. I meant to mention this in bug 1272098 but I see I forgot to do so.
Comment 14•8 years ago
|
||
Oops, sorry, comment #13 got delayed by a mid-air collision and so might only make sense a few comments earlier.
Assignee | ||
Comment 15•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #7) > > (In reply to Tom Tromey :tromey from comment #6) > > > > This seems reasonable. The dynamic lookup here means we'll have to be a bit > > more careful in our webpack config. Also I wonder what URLs are being > > passed in here -- if chrome: URLs then those won't work in content... > > > > I plan to make another patch to make sure we don't pass chrome:// urls here. As discussed with Tom on IRC yesterday, the properties files are currently only accessible via chrome:// URLs. We quickly discussed two options here: 1- Use require("raw!devtools/locale/inspector.properties") and add some logic in the raw loader that will prefix *.properties URLs with chrome://. This means we effectively keep using chrome URLs when loaded in chrome and we will need to figure out a way to make webpack find the properties files. 2- Change the way we install the *.properties so that we can access them from a resource:// URL. If we do so using the DevtoolsModules() helper in moz.build, this means we will end up having to require("raw!devtools/client/locales/en-US/inspector.properties"). We can use navigator.language to get the locale of the browser so we should be able to build the path here. However this means adding a moz.build file in the devtools/client/locales/en-US folder, which means the same would need to be done on all l10n repos, if that's even possible. Maybe I'm missing something, but solution 2 does not seem like the way to go. Unless someone has another option, can I go ahead with the first solution for now? Regarding the issue with webpack, maybe the LocalizationHelper could check if it's running in chrome. If yes it would prefix the paths with "chrome://", if not it would rewrite the paths to target devtools/client/locales/en-US ? (Not sure what our plans are for localizations of the webpack version of devtools, but I guess in the beginning it will be en-US only?)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 19•8 years ago
|
||
Cleaned up the current implementation, and pushed to try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=a129cd6b0aa5de0c0553c8e2c1f4fbcead169136, which is green after fixing a few tests depending directly on the internals of the LocalizationHelper. (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #11) > (In reply to Julian Descottes [:jdescottes] from comment #8) > > jryans: do you know the process to include external libraries such as the > > ones mentioned above? > > For license review, look over the license policy page[1]. Basically, file a > licensing team bug[2] to get a clear answer from them on how to proceed. > > Depending on the license, you may need to add the license to the > about:license page. > Thanks for the info! Opened Bug 1296600.
Comment 20•8 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #15) > Regarding the issue with webpack, maybe the LocalizationHelper could check > if it's running in chrome. I think it would be better to try pretty hard to isolate any chrome-vs-content dependencies elsewhere. It can be done in the loader or in more extreme situations by writing two modules, one for chrome and one for content, in devtools/shared/platform/. In this particular case the change can be made in loader-plugin-raw.jsm, I think. This isn't used in content; the solution for the content case is to document the new requirement(s) in the webpack bug (bug 1291049) so that the webpack config can be updated.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8782868 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8782869 -
Attachment is obsolete: true
Comment 22•8 years ago
|
||
mozreview-review |
Comment on attachment 8782397 [details] Bug 1294220 - part1: Move ellipsis character to a localized string in properties file; https://reviewboard.mozilla.org/r/72582/#review70796 ::: devtools/client/shared/l10n.js:7 (Diff revision 3) > > -const { Ci } = require("chrome"); > -const Services = require("Services"); > - > const { parsePropertiesFile } = require("devtools/client/shared/vendor/node-properties"); > const { vsprintf } = require("devtools/client/shared/vendor/sprintf"); Weird that these vendor libs are showing up in mozreview, but I see it's not part of the patch so no problem
Attachment #8782397 -
Flags: review?(bgrinstead) → review+
Comment 23•8 years ago
|
||
I think the ni? has been answered - let me know if not
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 24•8 years ago
|
||
Thanks for the review! I cleaned up the current patches and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=55188394d0a1 Will push them all for review if try is green. (In reply to Brian Grinstead [:bgrins] from comment #10) > (In reply to Julian Descottes [:jdescottes] from comment #8) > > I'm not sure what modifications were needed for these libs, but we should > document the update process - including the current version number, link to > the lib, license, and any patches needed for updates About this, I added UPGRADE text instructions. To be honest, these are small libraries, I don't really expect we will need to synchronize with the upstream versions. So I didn't take the time to create actual scripts to reapply automatically the changes I did here. If one of them becomes a pain to maintain, I guess we should take the time to rewrite it ourselves.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 31•8 years ago
|
||
mozreview-review |
Comment on attachment 8783434 [details] Bug 1294220 - part4: Add chrome:// protocol when loading raw properties files; https://reviewboard.mozilla.org/r/73248/#review71032 Thank you. This looks good.
Attachment #8783434 -
Flags: review?(ttromey) → review+
Comment 32•8 years ago
|
||
mozreview-review |
Comment on attachment 8783431 [details] Bug 1294220 - part2: Devtools shared l10n helper: remove dependency on Services.string; https://reviewboard.mozilla.org/r/73242/#review71036 ::: devtools/client/shared/l10n.js:16 (Diff revision 1) > * > * @param string stringBundleName > * The desired string bundle's name. > */ > function LocalizationHelper(stringBundleName) { > - loader.lazyGetter(this, "stringBundle", () => > + loader.lazyGetter(this, "properties", () => { We also need to get rid of loader.lazyGetter uses. It seems like this one could be easily done as a getter on LocalizationHelper itself.
Comment 33•8 years ago
|
||
mozreview-review |
Comment on attachment 8783435 [details] Bug 1294220 - part5: Remove the "chrome://" protocol when using LocalizationHelper; https://reviewboard.mozilla.org/r/73250/#review71038 Thank you. This patch is fine. I'm curious, though, how the locale-finding will actually work. LocalizationHelper doesn't currently seem to do this. What I mean is, the source refers to a file by one name, like "devtools/clientinspector.properties", but the file is actually "devtools/client/locales/en-US/inspector.properties". For the webpack case we can handle this by rewriting the paths, assuming perhaps that we only plan to support the one locale. But I don't understand how it works for the chrome case.
Attachment #8783435 -
Flags: review?(ttromey) → review+
Comment 34•8 years ago
|
||
mozreview-review |
Comment on attachment 8783433 [details] Bug 1294220 - part6: use standard getter instead of loader.lazyGetter in l10n.js; https://reviewboard.mozilla.org/r/73246/#review71170 r=me but this should be pulled into part 2 so that change is more self-contained
Attachment #8783433 -
Flags: review?(bgrinstead) → review+
Comment 35•8 years ago
|
||
mozreview-review |
Comment on attachment 8783432 [details] Bug 1294220 - part3: Escape % characters in localized strings; https://reviewboard.mozilla.org/r/73244/#review71172 Interesting, I guess the `%%` syntax is valid, but not required, for producing a single `%` with Services.strings. r=me once you bump the key names to it will get re-localized ::: devtools/client/locales/en-US/memory.properties:215 (Diff revision 1) > # that represents the root of the tree when inverted. > tree-item.root=(root) > > # LOCALIZATION NOTE (tree-item.percent): A percent of bytes or count displayed in the tree view. > -tree-item.percent=%S% > +# there are two "%" after %S to escape and display "%" > +tree-item.percent=%S%% These key names need to be bumped (tree-item.percent2 and table.percentage3)
Attachment #8783432 -
Flags: review?(bgrinstead)
Comment 36•8 years ago
|
||
mozreview-review |
Comment on attachment 8783431 [details] Bug 1294220 - part2: Devtools shared l10n helper: remove dependency on Services.string; https://reviewboard.mozilla.org/r/73242/#review71178
Comment 37•8 years ago
|
||
mozreview-review |
Comment on attachment 8783431 [details] Bug 1294220 - part2: Devtools shared l10n helper: remove dependency on Services.string; https://reviewboard.mozilla.org/r/73242/#review71180
Attachment #8783431 -
Flags: review?(bgrinstead) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 48•8 years ago
|
||
Thanks for the reviews Tom & Brian! (In reply to Brian Grinstead [:bgrins] from comment #35) > Comment on attachment 8783432 [details] > Bug 1294220 - part3: Escape % characters in localized strings; > > https://reviewboard.mozilla.org/r/73244/#review71172 > > Interesting, I guess the `%%` syntax is valid, but not required, for > producing a single `%` with Services.strings. r=me once you bump the key > names to it will get re-localized > > ::: devtools/client/locales/en-US/memory.properties:215 > (Diff revision 1) > > # that represents the root of the tree when inverted. > > tree-item.root=(root) > > > > # LOCALIZATION NOTE (tree-item.percent): A percent of bytes or count displayed in the tree view. > > -tree-item.percent=%S% > > +# there are two "%" after %S to escape and display "%" > > +tree-item.percent=%S%% > > These key names need to be bumped (tree-item.percent2 and table.percentage3) Good catch, thanks ! And yes, with Services.string, the % doesn't have to be escaped if it's the last character of the string. When in the middle of the string, we already had to escape it (for instance "profiler.bufferFull" in performance.properties). (In reply to Tom Tromey :tromey from comment #32) > Comment on attachment 8783431 [details] > > We also need to get rid of loader.lazyGetter uses. It seems like this one > could be easily done as a getter on LocalizationHelper itself. Sure, pushed a new patch for this :) (In reply to Tom Tromey :tromey from comment #33) > > I'm curious, though, how the locale-finding will actually work. > LocalizationHelper doesn't currently seem to do this. What I mean is, the > source refers to a file by one name, like > "devtools/clientinspector.properties", but the file is actually > "devtools/client/locales/en-US/inspector.properties". For the webpack case > we can handle this by rewriting the paths, assuming perhaps that we only > plan to support the one locale. But I don't understand how it works for the > chrome case. As discussed on IRC, using a chrome url should guarantee that the correct locale package is targeted, and this should be transparent for the "chrome" use case. For the "content" use case, we are fine with only having the en-US localization for now so we just have to configure webpack to map "devtools/locale/*.properties" to "devtools/client/locales/en-US/*.properties" (and "devtools-shared/locale/*.properties" to "devtools/shared/locales/en-US/*.properties")
Comment 49•8 years ago
|
||
mozreview-review |
Comment on attachment 8783432 [details] Bug 1294220 - part3: Escape % characters in localized strings; https://reviewboard.mozilla.org/r/73244/#review71236
Attachment #8783432 -
Flags: review?(bgrinstead) → review+
Assignee | ||
Comment 50•8 years ago
|
||
Pushed the last version to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4bdff5407a1, all green so far.
Comment 51•8 years ago
|
||
mozreview-review |
Comment on attachment 8783433 [details] Bug 1294220 - part6: use standard getter instead of loader.lazyGetter in l10n.js; https://reviewboard.mozilla.org/r/73246/#review71402 Thank you very much!
Attachment #8783433 -
Flags: review?(ttromey) → review+
Comment 52•8 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b79bba6f2091 part1: Move ellipsis character to a localized string in properties file;r=bgrins https://hg.mozilla.org/integration/autoland/rev/fc125c233fe0 part2: Devtools shared l10n helper: remove dependency on Services.string;r=bgrins https://hg.mozilla.org/integration/autoland/rev/f92cdcfe5d46 part3: Escape % characters in localized strings;r=bgrins https://hg.mozilla.org/integration/autoland/rev/fca9b2cc3f8f part4: Add chrome:// protocol when loading raw properties files;r=tromey https://hg.mozilla.org/integration/autoland/rev/1cccb1ca9c92 part5: Remove the "chrome://" protocol when using LocalizationHelper;r=tromey https://hg.mozilla.org/integration/autoland/rev/4d0ea69c57f6 part6: use standard getter instead of loader.lazyGetter in l10n.js;r=bgrins,tromey
Comment 53•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b79bba6f2091 https://hg.mozilla.org/mozilla-central/rev/fc125c233fe0 https://hg.mozilla.org/mozilla-central/rev/f92cdcfe5d46 https://hg.mozilla.org/mozilla-central/rev/fca9b2cc3f8f https://hg.mozilla.org/mozilla-central/rev/1cccb1ca9c92 https://hg.mozilla.org/mozilla-central/rev/4d0ea69c57f6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•