1062623 - Customization code should be using Console.jsm instead of it's own custom mechanism

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

The customization code (CustomizableUI.jsm, etc) uses it's own custom logging functions. Now days we have Log.jsm - it should use that, for:
* consistency with the rest of the newer parts of the codebase
* better logging functionality
* reduction of code

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Relevant code at /browser/components/customizableui

Current custom logging is defined in logging.js - we should remove this, and have the modules import Log.jsm and configure it as needed. See other examples in the tree for how to use, and conventions used.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Pro tip: DXR is a wonderful thing. http://dxr.mozilla.org/mozilla-central/source/ to explore/search the codebase. Search for a filename like "Log.jsm", and it'll take you there.

Comment 3

[Matthew Cook]

Attachment: patch: Remove custom logging code in favour of using Log.jsm

The changes here have been tested manually, I wasn't sure how to rig up a test for logging output so guidance there would be appreciated.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8529747 [details] [diff] [review]
patch: Remove custom logging code in favour of using Log.jsm

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

Just a couple of small fixups.

::: browser/components/customizableui/CustomizeMode.jsm
@@ +1030,5 @@
>            this.wrapToolbarItem(child, CustomizableUI.getPlaceForItem(child));
>          }
>        }
>      } catch (ex) {
> +      gLogger.error(ex, ex.stack);

Log.jsm automatically expands exceptions to also log their stack trace, so you don't need to pass in ex.stack

@@ +1769,5 @@
>  
>      try {
>        this._applyDrop(aEvent, targetArea, originArea, draggedItemId, targetNode);
>      } catch (ex) {
> +      gLogger.error(ex, ex.stack);

Ditto.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Fixed up those two nits for you, and landed on the fx-team branch - which gets merged into mozilla-central a couple of times a day.

https://hg.mozilla.org/integration/fx-team/rev/be8677c20b9f

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

This broke numerous tests, so backed out:
https://hg.mozilla.org/integration/fx-team/rev/f62dc60ec207

Matt: Looks like there's a few cases where ERROR is passed as a promise rejection handler in CustomizeMode.jsm. Should go through the rest of the files looking for that pattern too.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

For reference, you can run the relevant test suite by running:

  ./mach test browser/components/customizableui

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Opening this up for someone else to finish off. Think it just needs the thing in comment 6 addressed.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Oh, I just fixed this in bug 1242137 :-)