Closed
Bug 1062623
Opened 10 years ago
Closed 8 years ago
Customization code should be using Console.jsm instead of it's own custom mechanism
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: Unfocused, Assigned: jaws)
References
Details
(Whiteboard: [fixed by bug 1242137])
Attachments
(1 file)
42.95 KB,
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•10 years ago
|
||
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.
Reporter | ||
Comment 2•10 years ago
|
||
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.
Assignee: nobody → mgcook89
Status: NEW → ASSIGNED
Comment 3•10 years ago
|
||
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.
Attachment #8529747 -
Flags: review?(bmcbride)
Reporter | ||
Comment 4•10 years ago
|
||
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.
Attachment #8529747 -
Flags: review?(bmcbride) → review+
Reporter | ||
Comment 5•10 years ago
|
||
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
Reporter | ||
Updated•10 years ago
|
Flags: qe-verify-
Reporter | ||
Comment 6•10 years ago
|
||
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.
Flags: needinfo?(mgcook89)
Whiteboard: [good second bug][lang=js] → [good second bug][lang=js][backed out]
Reporter | ||
Comment 7•9 years ago
|
||
For reference, you can run the relevant test suite by running: ./mach test browser/components/customizableui
Reporter | ||
Comment 8•8 years ago
|
||
Opening this up for someone else to finish off. Think it just needs the thing in comment 6 addressed.
Assignee: mgcook89 → nobody
Mentor: bmcbride
Status: ASSIGNED → NEW
Flags: needinfo?(mgcook89)
OS: Windows 8.1 → All
Hardware: x86_64 → All
Assignee | ||
Comment 9•8 years ago
|
||
Oh, I just fixed this in bug 1242137 :-)
Assignee: nobody → jaws
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [good second bug][lang=js][backed out] → [fixed by bug 1242137]
Assignee | ||
Updated•8 years ago
|
Summary: Customization code should be using Log.jsm instead of it's own custom mechanism → Customization code should be using Console.jsm instead of it's own custom mechanism
You need to log in
before you can comment on or make changes to this bug.
Description
•