1258305 - Pull ViewHelpers.L10N, MultiL10N into their own module

Status: RESOLVED FIXED

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

Description

[Victor Porof [:vporof][:vp]]

ViewHelpers is a monster. Need to slim it down.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: l10n-pull.patch

Comment 2

[Victor Porof [:vporof][:vp]]

Files worthy to look at are ViewHelpers.jsm and devtools/client/shared/l10n.js
Everything else is busywork.

Comment 3

[Victor Porof [:vporof][:vp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfdbce1cb8de

Comment 4

[Victor Porof [:vporof][:vp]]

Attachment: v1 base

Comment 5

[Victor Porof [:vporof][:vp]]

Attachment: v1 busywork

Comment 6

[Victor Porof [:vporof][:vp]]

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

Comment 7

[Victor Porof [:vporof][:vp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d46ec09bcb8f

Comment 8

[Victor Porof [:vporof][:vp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cbf9fdf9d44

Comment 9

[Victor Porof [:vporof][:vp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67c6e7083883

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

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

Comment 11

[Victor Porof [:vporof][:vp]]

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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cc0858469277
https://hg.mozilla.org/mozilla-central/rev/b8b21810b2b3

Comment 13

[Tim Nguyen :ntim]

Marking addon-compat because it broke my 2 devtools addons.