From 4004c9cf44e6e64defc67e64dc3ca6591e7e27f2 Mon Sep 17 00:00:00 2001 From: Dave Townsend Date: Thu, 27 Mar 2014 11:35:14 -0700 Subject: [PATCH] Bug 986838: Add-on SDK modules should be listed after the add-on's own modules. r=fitzgen --- .../devtools/debugger/debugger-controller.js | 16 ------- browser/devtools/debugger/debugger-panes.js | 42 +++++++++++++++---- .../test/browser_dbg_addon-modules.js | 5 +++ .../test/browser_dbg_addon-sources.js | 6 +++ .../shared/widgets/SideMenuWidget.jsm | 15 ++++--- 5 files changed, 54 insertions(+), 30 deletions(-) diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index 758410db1982..733471f9b573 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -2175,22 +2175,6 @@ Object.defineProperties(window, { } }); -/** - * Helper method for parsing a resource URI, like - * `resource://gre/modules/commonjs/sdk/tabs.js`, and pulling out `sdk/tabs.js` - * if it's in the SDK, or `null` otherwise. - * - * @param string url - * @return string|null - */ -function getSDKModuleName(url) { - let match = (url || "").match(/^resource:\/\/gre\/modules\/commonjs\/(.*)/); - if (match) { - return match[1]; - } - return null; -} - /** * Helper method for debugging. * @param string diff --git a/browser/devtools/debugger/debugger-panes.js b/browser/devtools/debugger/debugger-panes.js index 52334fbeccdf..9085e97c3515 100644 --- a/browser/devtools/debugger/debugger-panes.js +++ b/browser/devtools/debugger/debugger-panes.js @@ -11,6 +11,11 @@ const SAMPLE_SIZE = 50; // no of lines const INDENT_COUNT_THRESHOLD = 5; // percentage const CHARACTER_LIMIT = 250; // line character limit +// Maps known URLs to friendly source group names +const KNOWN_SOURCE_GROUPS = { + "Add-on SDK": "resource://gre/modules/commonjs/", +}; + /** * Functions handling the sources UI. */ @@ -49,6 +54,15 @@ SourcesView.prototype = Heritage.extend(WidgetMethods, { showArrows: true }); + // Sort known source groups towards the end of the list + this.widget.groupSortPredicate = function(a, b) { + if ((a in KNOWN_SOURCE_GROUPS) == (b in KNOWN_SOURCE_GROUPS)) { + return a.localeCompare(b); + } + + return (a in KNOWN_SOURCE_GROUPS) ? 1 : -1; + }; + this.emptyText = L10N.getStr("noSourcesText"); this._blackBoxCheckboxTooltip = L10N.getStr("blackBoxCheckboxTooltip"); @@ -132,12 +146,6 @@ SourcesView.prototype = Heritage.extend(WidgetMethods, { let group = SourceUtils.getSourceGroup(url); let unicodeUrl = NetworkHelper.convertToUnicode(unescape(fullUrl)); - let sdkModuleName = getSDKModuleName(url); - if (sdkModuleName) { - label = sdkModuleName; - group = "Add-on SDK"; - } - let contents = document.createElement("label"); contents.className = "plain dbg-source-item"; contents.setAttribute("value", label); @@ -1560,7 +1568,17 @@ let SourceUtils = { return cachedLabel; } - let sourceLabel = this.trimUrl(aUrl); + let sourceLabel = null; + + for (let name of Object.keys(KNOWN_SOURCE_GROUPS)) { + if (aUrl.startsWith(KNOWN_SOURCE_GROUPS[name])) { + sourceLabel = aUrl.substring(KNOWN_SOURCE_GROUPS[name].length); + } + } + + if (!sourceLabel) { + sourceLabel = this.trimUrl(aUrl); + } let unicodeLabel = NetworkHelper.convertToUnicode(unescape(sourceLabel)); this._labelsCache.set(aUrl, unicodeLabel); return unicodeLabel; @@ -1583,14 +1601,20 @@ let SourceUtils = { try { // Use an nsIURL to parse all the url path parts. - let url = aUrl.split(" -> ").pop(); - var uri = Services.io.newURI(url, null, null).QueryInterface(Ci.nsIURL); + var uri = Services.io.newURI(aUrl, null, null).QueryInterface(Ci.nsIURL); } catch (e) { // This doesn't look like a url, or nsIURL can't handle it. return ""; } let groupLabel = uri.prePath; + + for (let name of Object.keys(KNOWN_SOURCE_GROUPS)) { + if (aUrl.startsWith(KNOWN_SOURCE_GROUPS[name])) { + groupLabel = name; + } + } + let unicodeLabel = NetworkHelper.convertToUnicode(unescape(groupLabel)); this._groupsCache.set(aUrl, unicodeLabel) return unicodeLabel; diff --git a/browser/devtools/debugger/test/browser_dbg_addon-modules.js b/browser/devtools/debugger/test/browser_dbg_addon-modules.js index e24c9801cecd..bac6fb41b97a 100644 --- a/browser/devtools/debugger/test/browser_dbg_addon-modules.js +++ b/browser/devtools/debugger/test/browser_dbg_addon-modules.js @@ -103,6 +103,11 @@ function testSources(expectSecondModule) { is(gTitle, "Debugger - Test add-on with JS Modules", "Saw the right toolbox title."); + let groups = gDebugger.document.querySelectorAll(".side-menu-widget-group-title .name"); + is(groups[0].value, "jar:", "Add-on bootstrap should be the first group"); + is(groups[1].value, "resource://browser_dbg_addon4", "Add-on code should be the second group"); + is(groups.length, 2, "Should be only two groups."); + deferred.resolve(); }); diff --git a/browser/devtools/debugger/test/browser_dbg_addon-sources.js b/browser/devtools/debugger/test/browser_dbg_addon-sources.js index ad6a0be985bf..3a455a63f26a 100644 --- a/browser/devtools/debugger/test/browser_dbg_addon-sources.js +++ b/browser/devtools/debugger/test/browser_dbg_addon-sources.js @@ -104,6 +104,12 @@ function testSources() { is(gTitle, "Debugger - browser_dbg_addon3", "Saw the right toolbox title."); + let groups = gDebugger.document.querySelectorAll(".side-menu-widget-group-title .name"); + is(groups[0].value, "jar:", "Add-on bootstrap should be the first group"); + is(groups[1].value, "resource://jid1-ami3akps3baaeg-at-jetpack", "Add-on code should be the second group"); + is(groups[2].value, "Add-on SDK", "Add-on SDK should be the third group"); + is(groups.length, 3, "Should be only three groups."); + deferred.resolve(); }); diff --git a/browser/devtools/shared/widgets/SideMenuWidget.jsm b/browser/devtools/shared/widgets/SideMenuWidget.jsm index 3a082411b3ca..dd6ab4b2ff2b 100644 --- a/browser/devtools/shared/widgets/SideMenuWidget.jsm +++ b/browser/devtools/shared/widgets/SideMenuWidget.jsm @@ -66,10 +66,15 @@ this.SideMenuWidget = function SideMenuWidget(aNode, aOptions={}) { SideMenuWidget.prototype = { /** - * Specifies if groups in this container should be sorted alphabetically. + * Specifies if groups in this container should be sorted. */ sortedGroups: true, + /** + * The comparator used to sort groups. + */ + groupSortPredicate: function(a, b) a.localeCompare(b), + /** * Specifies that the container viewport should be "stuck" to the * bottom. That is, the container is automatically scrolled down to @@ -342,7 +347,7 @@ SideMenuWidget.prototype = { }); this._groupsByName.set(aName, group); - group.insertSelfAt(this.sortedGroups ? group.findExpectedIndexForSelf() : -1); + group.insertSelfAt(this.sortedGroups ? group.findExpectedIndexForSelf(this.groupSortPredicate) : -1); return group; }, @@ -484,14 +489,14 @@ SideMenuGroup.prototype = { * @return number * The expected index. */ - findExpectedIndexForSelf: function() { + findExpectedIndexForSelf: function(sortPredicate) { let identifier = this.identifier; let groupsArray = this._orderedGroupElementsArray; for (let group of groupsArray) { let name = group.getAttribute("name"); - if (name > identifier && // Insertion sort at its best :) - !name.contains(identifier)) { // Least significat group should be last. + if (sortPredicate(name, identifier) > 0 && // Insertion sort at its best :) + !name.contains(identifier)) { // Least significant group should be last. return groupsArray.indexOf(group); } }