Closed Bug 1258305 Opened 4 years ago Closed 4 years ago

Pull ViewHelpers.L10N, MultiL10N into their own module

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)

defect

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: vporof, Assigned: vporof)

Details

(Keywords: addon-compat)

Attachments

(2 files, 1 obsolete file)

ViewHelpers is a monster. Need to slim it down.
Summary: Pull ViewHelpers.L10N, MultiL10N and Prefs into their own modules → Pull ViewHelpers.L10N, MultiL10N into their own module
Attached patch l10n-pull.patch (obsolete) — Splinter Review
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Attachment #8732790 - Flags: review?(jsantell)
Files worthy to look at are ViewHelpers.jsm and devtools/client/shared/l10n.js
Everything else is busywork.
Attached patch v1 baseSplinter Review
Attachment #8732790 - Attachment is obsolete: true
Attachment #8732790 - Flags: review?(jsantell)
Attachment #8732822 - Flags: review?(jsantell)
Attached patch v1 busyworkSplinter Review
Attachment #8732823 - Flags: review?(jsantell)
(In reply to Victor Porof [:vporof][:vp] from comment #2)
> Files worthy to look at are ViewHelpers.jsm and
> devtools/client/shared/l10n.js
> Everything else is busywork.

Split that giant patch into two smaller ones for easier review.
Attachment #8732822 - Flags: review?(jsantell) → review+
Comment on attachment 8732823 [details] [diff] [review]
v1 busywork

Review of attachment 8732823 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/layout/layout.js
@@ +9,5 @@
>  const {Cc, Ci, Cu} = require("chrome");
>  const {InplaceEditor, editableItem} =
>        require("devtools/client/shared/inplace-editor");
>  const {ReflowFront} = require("devtools/server/actors/layout");
> +const {Localization} = require("devtools/client/shared/l10n");

nit spacing

::: devtools/client/netmonitor/netmonitor-view.js
@@ +21,5 @@
>  
>  const {ToolSidebar} = require("devtools/client/framework/sidebar");
>  const {Tooltip} = require("devtools/client/shared/widgets/Tooltip");
>  const DevToolsUtils = require("devtools/shared/DevToolsUtils");
> +const {Localization} = require("devtools/client/shared/l10n");

nit spacing

::: devtools/client/responsive.html/components/utils/l10n.js
@@ +1,1 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public

This file might be redundant now

::: devtools/client/shared/source-utils.js
@@ +5,5 @@
>  
>  const { URL } = require("sdk/url");
> +const { Localization } = require("devtools/client/shared/l10n");
> +
> +const l10n = new Localization("chrome://devtools/locale/components.properties");

Lower case `l10n` was used because the previous import was `L10N` -- maybe this should always be uppercased now for consistency across tools

::: devtools/client/shared/test/browser_flame-graph-04.js
@@ +3,5 @@
>  
>  // Tests that text metrics in the flame graph widget work properly.
>  
>  var HTML_NS = "http://www.w3.org/1999/xhtml";
> +var {Localization} = require("devtools/client/shared/l10n");

nit `{ Localization }`

::: devtools/client/shared/widgets/VariablesViewController.jsm
@@ +12,5 @@
>  Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm");
>  var {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
>  var Services = require("Services");
>  var promise = require("promise");
> +var {Localization} = require("devtools/client/shared/l10n");

nit spacing

::: devtools/client/storage/ui.js
@@ +6,5 @@
>  "use strict";
>  
>  const {Cu} = require("chrome");
>  const EventEmitter = require("devtools/shared/event-emitter");
> +const {Localization} = require("devtools/client/shared/l10n");

nit spacing
Attachment #8732823 - Flags: review?(jsantell) → review+
https://hg.mozilla.org/integration/fx-team/rev/cc08584692775437121919c69adcd4fc0cfb97f7
Bug 1258305 - Pull ViewHelpers.L10N, MultiL10N into their own module, r=jsantell

https://hg.mozilla.org/integration/fx-team/rev/b8b21810b2b3b42a90dfda07c3ff0ecf1a030727
Bug 1258305 - Update all occurences of ViewHelpers.L10N and MultiL10N to use the new module, r=jsantell
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/cc0858469277
https://hg.mozilla.org/mozilla-central/rev/b8b21810b2b3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Marking addon-compat because it broke my 2 devtools addons.
Keywords: addon-compat
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.