From 8d4199e39ac6a05b84c005c8da73ebdd7e176439 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Fri, 25 Jan 2013 18:14:53 +0200 Subject: [PATCH 01/15] Bug 834721 - Use plurals for toolbox button tooltip; r=paul --- browser/devtools/shared/DeveloperToolbar.jsm | 17 ++++++++++++++++- .../browser_toolbar_webconsole_errors_count.js | 2 +- .../chrome/browser/devtools/toolbox.properties | 8 +++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/browser/devtools/shared/DeveloperToolbar.jsm b/browser/devtools/shared/DeveloperToolbar.jsm index 13a9ee67acf1..a98033ce31e2 100644 --- a/browser/devtools/shared/DeveloperToolbar.jsm +++ b/browser/devtools/shared/DeveloperToolbar.jsm @@ -27,6 +27,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "CmdCommands", XPCOMUtils.defineLazyModuleGetter(this, "PageErrorListener", "resource://gre/modules/devtools/WebConsoleUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "PluralForm", + "resource://gre/modules/PluralForm.jsm"); + XPCOMUtils.defineLazyGetter(this, "prefBranch", function() { let prefService = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefService); @@ -622,7 +625,19 @@ function DT__updateErrorsCount(aChangedTabId) let warnings = this._warningsCount[tabId]; let btn = this._errorCounterButton; if (errors) { - let tooltiptext = toolboxStrings.formatStringFromName("toolboxDockButtons.errorsCount.tooltip", [errors, warnings], 2); + let errorsText = toolboxStrings + .GetStringFromName("toolboxToggleButton.errorsCount"); + errorsText = PluralForm.get(errors, errorsText); + + let warningsText = toolboxStrings + .GetStringFromName("toolboxToggleButton.warningsCount"); + warningsText = PluralForm.get(warnings, warningsText); + + let tooltiptext = toolboxStrings + .formatStringFromName("toolboxToggleButton.tooltiptext", + [errors, errorsText, warnings, + warningsText], 4); + btn.setAttribute("error-count", errors); btn.setAttribute("tooltiptext", tooltiptext); } else { diff --git a/browser/devtools/shared/test/browser_toolbar_webconsole_errors_count.js b/browser/devtools/shared/test/browser_toolbar_webconsole_errors_count.js index b3d6b291568f..85d8d825a9f8 100644 --- a/browser/devtools/shared/test/browser_toolbar_webconsole_errors_count.js +++ b/browser/devtools/shared/test/browser_toolbar_webconsole_errors_count.js @@ -44,7 +44,7 @@ function test() { function getTooltipValues() { let matches = webconsole.getAttribute("tooltiptext") - .match(/(\d+) errors, (\d+) warnings/); + .match(/(\d+) errors?, (\d+) warnings?/); return matches ? [matches[1], matches[2]] : [0, 0]; } diff --git a/browser/locales/en-US/chrome/browser/devtools/toolbox.properties b/browser/locales/en-US/chrome/browser/devtools/toolbox.properties index f5c08f442a24..01264cad4d9d 100644 --- a/browser/locales/en-US/chrome/browser/devtools/toolbox.properties +++ b/browser/locales/en-US/chrome/browser/devtools/toolbox.properties @@ -1,4 +1,10 @@ toolboxDockButtons.bottom.tooltip=Dock to bottom of browser window toolboxDockButtons.side.tooltip=Dock to side of browser window toolboxDockButtons.window.tooltip=Show in separate window -toolboxDockButtons.errorsCount.tooltip=%S errors, %S warnings.\nClick to toggle developer tools. \ No newline at end of file + +# LOCALIZATION NOTE (toolboxToggleButton): These strings are used for the button +# that allows users to open/close the developer tools. You can find this button +# on the developer toolbar. +toolboxToggleButton.errorsCount=error;errors +toolboxToggleButton.warningsCount=warning;warnings +toolboxToggleButton.tooltiptext=%S %S, %S %S.\nClick to toggle the developer tools. From d52d4ca35cd965f1ceedaf0ae2323af9214d0990 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Thu, 31 Jan 2013 19:19:06 +0200 Subject: [PATCH 02/15] Bug 720180 - console.log('foo'); console.error('foo'); should not be considered as a repeat; r=past --- browser/devtools/webconsole/test/Makefile.in | 2 + .../browser_repeated_messages_accuracy.js | 109 ++++++++++++++++ ...browser_webconsole_bug_642108_pruneTest.js | 15 ++- .../test/test-repeated-messages.html | 29 +++++ browser/devtools/webconsole/webconsole.js | 117 +++++------------- 5 files changed, 186 insertions(+), 86 deletions(-) create mode 100644 browser/devtools/webconsole/test/browser_repeated_messages_accuracy.js create mode 100644 browser/devtools/webconsole/test/test-repeated-messages.html diff --git a/browser/devtools/webconsole/test/Makefile.in b/browser/devtools/webconsole/test/Makefile.in index 79ce488d665b..4afe6970598c 100644 --- a/browser/devtools/webconsole/test/Makefile.in +++ b/browser/devtools/webconsole/test/Makefile.in @@ -114,6 +114,7 @@ MOCHITEST_BROWSER_FILES = \ browser_bug_638949_copy_link_location.js \ browser_output_longstring_expand.js \ browser_netpanel_longstring_expand.js \ + browser_repeated_messages_accuracy.js \ head.js \ $(NULL) @@ -202,6 +203,7 @@ MOCHITEST_BROWSER_FILES += \ test_bug_770099_bad_policy_uri.html^headers^ \ test-result-format-as-string.html \ test-bug-737873-mixedcontent.html \ + test-repeated-messages.html \ $(NULL) include $(topsrcdir)/config/rules.mk diff --git a/browser/devtools/webconsole/test/browser_repeated_messages_accuracy.js b/browser/devtools/webconsole/test/browser_repeated_messages_accuracy.js new file mode 100644 index 000000000000..c28a212e19e1 --- /dev/null +++ b/browser/devtools/webconsole/test/browser_repeated_messages_accuracy.js @@ -0,0 +1,109 @@ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +// Tests that makes sure messages are not considered repeated when coming from +// different lines of code, or from different severities, etc. +// See bugs 720180 and 800510. + +const TEST_URI = "http://example.com/browser/browser/devtools/webconsole/test/test-repeated-messages.html"; + +function test() { + addTab(TEST_URI); + browser.addEventListener("load", function onLoad() { + browser.removeEventListener("load", onLoad, true); + openConsole(null, consoleOpened); + }, true); +} + +function consoleOpened(hud) { + // Check that css warnings are not coalesced if they come from different lines. + waitForSuccess({ + name: "css warnings displayed", + validatorFn: function() + { + return hud.outputNode.querySelectorAll(".webconsole-msg-cssparser") + .length == 2; + }, + successFn: testCSSRepeats.bind(null, hud), + failureFn: finishTest, + }); +} + +function repeatCountForNode(aNode) { + return aNode.querySelector(".webconsole-msg-repeat").getAttribute("value"); +} + +function testCSSRepeats(hud) { + let msgs = hud.outputNode.querySelectorAll(".webconsole-msg-cssparser"); + is(repeatCountForNode(msgs[0]), 1, "no repeats for the first css warning"); + is(repeatCountForNode(msgs[1]), 1, "no repeats for the second css warning"); + + browser.addEventListener("load", function onLoad() { + browser.removeEventListener("load", onLoad, true); + testAfterReload(hud); + }, true); + content.location.reload(); +} + +function testAfterReload(hud) { + waitForSuccess({ + name: "message repeats increased", + validatorFn: function() + { + return hud.outputNode.querySelector(".webconsole-msg-repeat") + .getAttribute("value") == 2; + }, + successFn: testCSSRepeatsAfterReload.bind(null, hud), + failureFn: finishTest, + }); +} + +function testCSSRepeatsAfterReload(hud) { + let msgs = hud.outputNode.querySelectorAll(".webconsole-msg-cssparser"); + is(msgs.length, 2, "two css warnings after reload"); + is(repeatCountForNode(msgs[0]), 2, "two repeats for the first css warning"); + is(repeatCountForNode(msgs[1]), 2, "two repeats for the second css warning"); + + hud.jsterm.clearOutput(); + content.wrappedJSObject.testConsole(); + + waitForSuccess({ + name: "console API messages displayed", + validatorFn: function() + { + return hud.outputNode.querySelectorAll(".webconsole-msg-console") + .length == 3; + }, + successFn: testConsoleRepeats.bind(null, hud), + failureFn: finishTest, + }); +} + +function testConsoleRepeats(hud) { + let msgs = hud.outputNode.querySelectorAll(".webconsole-msg-console"); + is(repeatCountForNode(msgs[0]), 2, "repeats for the first console message"); + is(repeatCountForNode(msgs[1]), 1, + "no repeats for the second console log message"); + is(repeatCountForNode(msgs[2]), 1, "no repeats for the console.error message"); + + hud.jsterm.clearOutput(); + hud.jsterm.execute("undefined"); + content.console.log("undefined"); + + waitForSuccess({ + name: "messages displayed", + validatorFn: function() + { + return hud.outputNode.querySelector(".webconsole-msg-console"); + }, + successFn: function() { + is(hud.outputNode.childNodes.length, 3, + "correct number of messages displayed"); + executeSoon(finishTest); + }, + failureFn: finishTest, + }); +} diff --git a/browser/devtools/webconsole/test/browser_webconsole_bug_642108_pruneTest.js b/browser/devtools/webconsole/test/browser_webconsole_bug_642108_pruneTest.js index f7672c9c41f3..8dd592e327af 100644 --- a/browser/devtools/webconsole/test/browser_webconsole_bug_642108_pruneTest.js +++ b/browser/devtools/webconsole/test/browser_webconsole_bug_642108_pruneTest.js @@ -55,8 +55,14 @@ function testCSSPruning(hudRef) { }, successFn: function() { - ok(!hudRef.ui._cssNodes["css log x"], + is(Object.keys(hudRef.ui._cssNodes).length, LOG_LIMIT, "repeated nodes pruned from cssNodes"); + + let msg = hudRef.outputNode.querySelector(".webconsole-msg-cssparser " + + ".webconsole-msg-repeat"); + is(msg.getAttribute("value"), 1, + "repeated nodes pruned from cssNodes (confirmed)"); + finishTest(); }, failureFn: finishTest, @@ -66,7 +72,12 @@ function testCSSPruning(hudRef) { name: "repeated nodes in cssNodes", validatorFn: function() { - return hudRef.ui._cssNodes["css log x"]; + let msg = hudRef.outputNode.querySelector(".webconsole-msg-cssparser " + + ".webconsole-msg-repeat"); + if (msg) { + console.debug(msg, msg.getAttribute("value")); + } + return msg && msg.getAttribute("value") == 5; }, successFn: function() { diff --git a/browser/devtools/webconsole/test/test-repeated-messages.html b/browser/devtools/webconsole/test/test-repeated-messages.html new file mode 100644 index 000000000000..4e21a69d6c09 --- /dev/null +++ b/browser/devtools/webconsole/test/test-repeated-messages.html @@ -0,0 +1,29 @@ + + + + + Test for bugs 720180 and 800510 + + + + + +

Hello world!

+ + + diff --git a/browser/devtools/webconsole/webconsole.js b/browser/devtools/webconsole/webconsole.js index 3add8ec68928..eb9173151b5a 100644 --- a/browser/devtools/webconsole/webconsole.js +++ b/browser/devtools/webconsole/webconsole.js @@ -867,83 +867,45 @@ WebConsoleFrame.prototype = { }, /** - * Filter the css node from the output node if it is a repeat. CSS messages - * are merged with previous messages if they occurred in the past. + * Filter the message node from the output if it is a repeat. * + * @private * @param nsIDOMNode aNode * The message node to be filtered or not. * @returns boolean * True if the message is filtered, false otherwise. */ - filterRepeatedCSS: function WCF_filterRepeatedCSS(aNode) + _filterRepeatedMessage: function WCF__filterRepeatedMessage(aNode) { - // childNodes[2] is the description node containing the text of the message. - let description = aNode.childNodes[2].textContent; - let location; + let repeatNode = aNode.getElementsByClassName("webconsole-msg-repeat")[0]; + let uid = repeatNode._uid; + let dupeNode = null; - // childNodes[4] represents the location (source URL) of the error message. - // The full source URL is stored in the title attribute. - if (aNode.childNodes[4]) { - // browser_webconsole_bug_595934_message_categories.js - location = aNode.childNodes[4].getAttribute("title"); + if (aNode.classList.contains("webconsole-msg-cssparser")) { + dupeNode = this._cssNodes[uid]; + if (!dupeNode) { + this._cssNodes[uid] = aNode; + } } - else { - location = ""; - } - - let dupe = this._cssNodes[description + location]; - if (!dupe) { - // no matching nodes - this._cssNodes[description + location] = aNode; - return false; - } - - this.mergeFilteredMessageNode(dupe, aNode); - - return true; - }, - - /** - * Filter the console node from the output node if it is a repeat. Console - * messages are filtered from the output if they match the immediately - * preceding message that came from the same source. The output node's - * last occurrence should have its timestamp updated. - * - * @param nsIDOMNode aNode - * The message node to be filtered or not. - * @return boolean - * True if the message is filtered, false otherwise. - */ - filterRepeatedConsole: function WCF_filterRepeatedConsole(aNode) - { - let lastMessage = this.outputNode.lastChild; - - if (!lastMessage) { - return false; - } - - let body = aNode.querySelector(".webconsole-msg-body"); - let lastBody = lastMessage.querySelector(".webconsole-msg-body"); - - if (aNode.classList.contains("webconsole-msg-inspector")) { - return false; - } - - if (!body || !lastBody) { - return false; - } - - if (body.textContent == lastBody.textContent) { - let loc = aNode.querySelector(".webconsole-location"); - let lastLoc = lastMessage.querySelector(".webconsole-location"); - - if (loc && lastLoc) { - if (loc.getAttribute("value") !== lastLoc.getAttribute("value")) { - return false; - } + else if (!aNode.classList.contains("webconsole-msg-network") && + !aNode.classList.contains("webconsole-msg-inspector") && + (aNode.classList.contains("webconsole-msg-console") || + aNode.classList.contains("webconsole-msg-exception") || + aNode.classList.contains("webconsole-msg-error"))) { + let lastMessage = this.outputNode.lastChild; + if (!lastMessage) { + return false; } - this.mergeFilteredMessageNode(lastMessage, aNode); + let lastRepeatNode = lastMessage + .getElementsByClassName("webconsole-msg-repeat")[0]; + if (lastRepeatNode._uid == uid) { + dupeNode = lastMessage; + } + } + + if (dupeNode) { + this.mergeFilteredMessageNode(dupeNode, aNode); return true; } @@ -1880,18 +1842,7 @@ WebConsoleFrame.prototype = { let isFiltered = this.filterMessageNode(node); - let isRepeated = false; - if (node.classList.contains("webconsole-msg-cssparser")) { - isRepeated = this.filterRepeatedCSS(node); - } - - if (!isRepeated && - !node.classList.contains("webconsole-msg-network") && - (node.classList.contains("webconsole-msg-console") || - node.classList.contains("webconsole-msg-exception") || - node.classList.contains("webconsole-msg-error"))) { - isRepeated = this.filterRepeatedConsole(node); - } + let isRepeated = this._filterRepeatedMessage(node); let lastVisible = !isRepeated && !isFiltered; if (!isRepeated) { @@ -2066,12 +2017,8 @@ WebConsoleFrame.prototype = { } if (aNode.classList.contains("webconsole-msg-cssparser")) { - let desc = aNode.childNodes[2].textContent; - let location = ""; - if (aNode.childNodes[4]) { - location = aNode.childNodes[4].getAttribute("title"); - } - delete this._cssNodes[desc + location]; + let repeatNode = aNode.getElementsByClassName("webconsole-msg-repeat")[0]; + delete this._cssNodes[repeatNode._uid]; } else if (aNode._connectionId && aNode.classList.contains("webconsole-msg-network")) { @@ -2210,6 +2157,8 @@ WebConsoleFrame.prototype = { let repeatNode = this.document.createElementNS(XUL_NS, "label"); repeatNode.setAttribute("value", "1"); repeatNode.classList.add("webconsole-msg-repeat"); + repeatNode._uid = [bodyNode.textContent, aCategory, aSeverity, aLevel, + aSourceURL, aSourceLine].join(":"); repeatContainer.appendChild(repeatNode); // Create the timestamp. From 0700540cb1f053d13541f1feebb129130157ca92 Mon Sep 17 00:00:00 2001 From: Heather Arthur Date: Tue, 5 Feb 2013 17:01:37 +0000 Subject: [PATCH 03/15] Bug 838108 - Enable style editor chrome debugging; r=paul --- browser/devtools/framework/ToolDefinitions.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/devtools/framework/ToolDefinitions.jsm b/browser/devtools/framework/ToolDefinitions.jsm index 40cf52558c11..582142243fbc 100644 --- a/browser/devtools/framework/ToolDefinitions.jsm +++ b/browser/devtools/framework/ToolDefinitions.jsm @@ -135,7 +135,7 @@ let styleEditorDefinition = { tooltip: l10n("ToolboxStyleEditor.tooltip", styleEditorStrings), isTargetSupported: function(target) { - return target.isLocalTab; + return !target.isRemote; }, build: function(iframeWindow, toolbox) { From 93010c047ec9cc24db47f817154f3c79a892bf34 Mon Sep 17 00:00:00 2001 From: Robert Strong Date: Tue, 5 Feb 2013 10:48:00 -0800 Subject: [PATCH 04/15] Bug 753730 - Intermittent test_0110_general_svc.js failure in head_update.js | UPDATE TYPE complete. r=bbondy --- toolkit/mozapps/update/test/unit/head_update.js.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/toolkit/mozapps/update/test/unit/head_update.js.in b/toolkit/mozapps/update/test/unit/head_update.js.in index 1a8000c76d17..5bfa6c6cb08a 100644 --- a/toolkit/mozapps/update/test/unit/head_update.js.in +++ b/toolkit/mozapps/update/test/unit/head_update.js.in @@ -1242,6 +1242,8 @@ function checkUpdateLogContents(aCompareLogFile) { // Skip the source/destination lines since they contain absolute paths. updateLogContents = updateLogContents.replace(/SOURCE DIRECTORY.*/g, ""); updateLogContents = updateLogContents.replace(/DESTINATION DIRECTORY.*/g, ""); + // Skip lines that log failed attempts to open the callback executable. + updateLogContents = updateLogContents.replace(/NS_main: callback app open attempt .*/g, ""); if (gSwitchApp) { // Remove the lines which contain absolute paths updateLogContents = updateLogContents.replace(/^Begin moving.*$/mg, ""); From 07df47a9ace42da5928666c0ca97a4fe58be8fdf Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Tue, 5 Feb 2013 18:44:41 +0200 Subject: [PATCH 05/15] Bug 720180 - Fix exception in the browser_webconsole_bug_588730_text_node_insertion.js test; r=past --- browser/devtools/webconsole/webconsole.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/browser/devtools/webconsole/webconsole.js b/browser/devtools/webconsole/webconsole.js index eb9173151b5a..b4be02d9e4bd 100644 --- a/browser/devtools/webconsole/webconsole.js +++ b/browser/devtools/webconsole/webconsole.js @@ -878,6 +878,10 @@ WebConsoleFrame.prototype = { _filterRepeatedMessage: function WCF__filterRepeatedMessage(aNode) { let repeatNode = aNode.getElementsByClassName("webconsole-msg-repeat")[0]; + if (!repeatNode) { + return false; + } + let uid = repeatNode._uid; let dupeNode = null; @@ -899,7 +903,7 @@ WebConsoleFrame.prototype = { let lastRepeatNode = lastMessage .getElementsByClassName("webconsole-msg-repeat")[0]; - if (lastRepeatNode._uid == uid) { + if (lastRepeatNode && lastRepeatNode._uid == uid) { dupeNode = lastMessage; } } @@ -2018,7 +2022,9 @@ WebConsoleFrame.prototype = { if (aNode.classList.contains("webconsole-msg-cssparser")) { let repeatNode = aNode.getElementsByClassName("webconsole-msg-repeat")[0]; - delete this._cssNodes[repeatNode._uid]; + if (repeatNode && repeatNode._uid) { + delete this._cssNodes[repeatNode._uid]; + } } else if (aNode._connectionId && aNode.classList.contains("webconsole-msg-network")) { From 4b2df60765a040075f9a9e440161a8e119a2fd8b Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Tue, 29 Jan 2013 17:05:17 +0000 Subject: [PATCH 06/15] Bug 700061 - Entirely remove the Style Inspector unmatched selectors --- .../devtools/styleinspector/CssHtmlTree.jsm | 150 +---------- browser/devtools/styleinspector/CssLogic.jsm | 252 +----------------- .../test/browser_bug683672.html | 6 +- .../styleinspector/test/browser_bug683672.js | 23 -- .../test/browser_styleinspector.js | 1 - .../browser/devtools/styleinspector.dtd | 15 +- .../devtools/styleinspector.properties | 1 - 7 files changed, 23 insertions(+), 425 deletions(-) diff --git a/browser/devtools/styleinspector/CssHtmlTree.jsm b/browser/devtools/styleinspector/CssHtmlTree.jsm index f8f517bcb77d..678a586eae91 100644 --- a/browser/devtools/styleinspector/CssHtmlTree.jsm +++ b/browser/devtools/styleinspector/CssHtmlTree.jsm @@ -216,9 +216,8 @@ XPCOMUtils.defineLazyGetter(this, "clipboardHelper", function() { }); CssHtmlTree.prototype = { - // Cache the list of properties that have matched and unmatched properties. + // Cache the list of properties that match the selected element. _matchedProperties: null, - _unmatchedProperties: null, htmlComplete: false, @@ -253,7 +252,6 @@ CssHtmlTree.prototype = { highlight: function CssHtmlTree_highlight(aElement) { this.viewedElement = aElement; - this._unmatchedProperties = null; this._matchedProperties = null; if (!aElement) { @@ -288,7 +286,7 @@ CssHtmlTree.prototype = { if (propView.visible) { this.numVisibleProperties++; } - propView.refreshAllSelectors(); + propView.refreshMatchedSelectors(); this.propertyViews.push(propView); }.bind(this), onDone: function() { @@ -439,41 +437,6 @@ CssHtmlTree.prototype = { return this._matchedProperties; }, - /** - * Check if a property has unmatched selectors. Result is cached. - * - * @param {string} aProperty the name of the property you want to check. - * @return {boolean} true if the property has unmatched selectors, false - * otherwise. - */ - hasUnmatchedSelectors: function CssHtmlTree_hasUnmatchedSelectors(aProperty) - { - // Initially check all of the properties that return false for - // hasMatchedSelectors(). This speeds-up the UI. - if (!this._unmatchedProperties) { - let properties = []; - CssHtmlTree.propertyNames.forEach(function(aName) { - if (!this.matchedProperties[aName]) { - properties.push(aName); - } - }, this); - - if (properties.indexOf(aProperty) == -1) { - properties.push(aProperty); - } - - this._unmatchedProperties = this.cssLogic.hasUnmatchedSelectors(properties); - } - - // Lazy-get the result for properties we do not have cached. - if (!(aProperty in this._unmatchedProperties)) { - let result = this.cssLogic.hasUnmatchedSelectors([aProperty]); - this._unmatchedProperties[aProperty] = result[aProperty]; - } - - return this._unmatchedProperties[aProperty]; - }, - /** * Create a context menu. */ @@ -777,33 +740,15 @@ PropertyView.prototype = { // Are matched rules expanded? matchedExpanded: false, - // Are unmatched rules expanded? - unmatchedExpanded: false, - - // Unmatched selector table - unmatchedSelectorTable: null, - // Matched selector container matchedSelectorsContainer: null, // Matched selector expando matchedExpander: null, - // Unmatched selector expando - unmatchedExpander: null, - - // Unmatched selector container - unmatchedSelectorsContainer: null, - - // Unmatched title block - unmatchedTitleBlock: null, - // Cache for matched selector views _matchedSelectorViews: null, - // Cache for unmatched selector views - _unmatchedSelectorViews: null, - // The previously selected element used for the selector view caches prevViewedElement: null, @@ -834,14 +779,6 @@ PropertyView.prototype = { return this.name in this.tree.matchedProperties; }, - /** - * Does the property have any unmatched selectors? - */ - get hasUnmatchedSelectors() - { - return this.name in this.tree.hasUnmatchedSelectors; - }, - /** * Should this property be visible? */ @@ -969,7 +906,6 @@ PropertyView.prototype = { if (this.prevViewedElement != this.tree.viewedElement) { this._matchedSelectorViews = null; - this._unmatchedSelectorViews = null; this.prevViewedElement = this.tree.viewedElement; } @@ -983,7 +919,7 @@ PropertyView.prototype = { this.tree.numVisibleProperties++; this.valueNode.textContent = this.propertyInfo.value; - this.refreshAllSelectors(); + this.refreshMatchedSelectors(); }, /** @@ -1010,51 +946,6 @@ PropertyView.prototype = { } }, - /** - * Refresh the panel unmatched rules. - */ - refreshUnmatchedSelectors: function PropertyView_refreshUnmatchedSelectors() - { - let hasMatchedSelectors = this.hasMatchedSelectors; - - this.unmatchedSelectorTable.hidden = !this.unmatchedExpanded; - - if (hasMatchedSelectors) { - this.unmatchedSelectorsContainer.hidden = !this.matchedExpanded || - !this.hasUnmatchedSelectors; - this.unmatchedTitleBlock.hidden = false; - } else { - this.unmatchedSelectorsContainer.hidden = !this.unmatchedExpanded; - this.unmatchedTitleBlock.hidden = true; - } - - if (this.unmatchedExpanded && this.hasUnmatchedSelectors) { - CssHtmlTree.processTemplate(this.templateUnmatchedSelectors, - this.unmatchedSelectorTable, this); - if (!hasMatchedSelectors) { - this.matchedExpander.setAttribute("open", ""); - this.unmatchedSelectorTable.classList.add("only-unmatched"); - } else { - this.unmatchedExpander.setAttribute("open", ""); - this.unmatchedSelectorTable.classList.remove("only-unmatched"); - } - } else { - if (!hasMatchedSelectors) { - this.matchedExpander.removeAttribute("open"); - } - this.unmatchedExpander.removeAttribute("open"); - this.unmatchedSelectorTable.innerHTML = ""; - } - }, - - /** - * Refresh the panel matched and unmatched rules - */ - refreshAllSelectors: function PropertyView_refreshAllSelectors() - { - this.refreshMatchedSelectors(); - }, - /** * Provide access to the matched SelectorViews that we are currently * displaying. @@ -1072,23 +963,6 @@ PropertyView.prototype = { return this._matchedSelectorViews; }, - /** - * Provide access to the unmatched SelectorViews that we are currently - * displaying. - */ - get unmatchedSelectorViews() - { - if (!this._unmatchedSelectorViews) { - this._unmatchedSelectorViews = []; - this.propertyInfo.unmatchedSelectors.forEach( - function unmatchedSelectorViews_convert(aSelectorInfo) { - this._unmatchedSelectorViews.push(new SelectorView(this.tree, aSelectorInfo)); - }, this); - } - - return this._unmatchedSelectorViews; - }, - /** * The action when a user expands matched selectors. * @@ -1098,17 +972,7 @@ PropertyView.prototype = { matchedExpanderClick: function PropertyView_matchedExpanderClick(aEvent) { this.matchedExpanded = !this.matchedExpanded; - this.refreshAllSelectors(); - aEvent.preventDefault(); - }, - - /** - * The action when a user expands unmatched selectors. - */ - unmatchedSelectorsClick: function PropertyView_unmatchedSelectorsClick(aEvent) - { - this.unmatchedExpanded = !this.unmatchedExpanded; - this.refreshUnmatchedSelectors(); + this.refreshMatchedSelectors(); aEvent.preventDefault(); }, @@ -1145,11 +1009,11 @@ function SelectorView(aTree, aSelectorInfo) * @see CssLogic.STATUS */ SelectorView.STATUS_NAMES = [ - // "Unmatched", "Parent Match", "Matched", "Best Match" + // "Parent Match", "Matched", "Best Match" ]; SelectorView.CLASS_NAMES = [ - "unmatched", "parentmatch", "matched", "bestmatch" + "parentmatch", "matched", "bestmatch" ]; SelectorView.prototype = { @@ -1170,7 +1034,7 @@ SelectorView.prototype = { for (let status in CssLogic.STATUS) { let i = CssLogic.STATUS[status]; - if (i > -1) { + if (i > CssLogic.STATUS.UNMATCHED) { let value = CssHtmlTree.l10n("rule.status." + status); // Replace normal spaces with non-breaking spaces SelectorView.STATUS_NAMES[i] = value.replace(/ /g, '\u00A0'); diff --git a/browser/devtools/styleinspector/CssLogic.jsm b/browser/devtools/styleinspector/CssLogic.jsm index 9f9aa994c199..fd078c89f80c 100644 --- a/browser/devtools/styleinspector/CssLogic.jsm +++ b/browser/devtools/styleinspector/CssLogic.jsm @@ -23,8 +23,7 @@ * standard DOM API, but more inline with the definition in the spec. * * - CssPropertyInfo contains style information for a single property for the - * highlighted element. It divides the CSS rules on the page into matched and - * unmatched rules. + * highlighted element. * - CssSelectorInfo is a wrapper around CssSelector, which adds sorting with * reference to the selected element. */ @@ -120,13 +119,11 @@ CssLogic.prototype = { // processMatchedSelectors(). _passId: 0, - // Used for tracking matched CssSelector objects, such that we can skip them - // in processUnmatchedSelectors(). + // Used for tracking matched CssSelector objects. _matchId: 0, _matchedRules: null, _matchedSelectors: null, - _unmatchedSelectors: null, domUtils: Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils), @@ -142,7 +139,6 @@ CssLogic.prototype = { this._sheetsCached = false; this._matchedRules = null; this._matchedSelectors = null; - this._unmatchedSelectors = null; }, /** @@ -177,7 +173,6 @@ CssLogic.prototype = { this._matchedRules = null; this._matchedSelectors = null; - this._unmatchedSelectors = null; let win = this.viewedDocument.defaultView; this._computedStyle = win.getComputedStyle(this.viewedElement, ""); }, @@ -220,7 +215,6 @@ CssLogic.prototype = { if (needFullUpdate) { this._matchedRules = null; this._matchedSelectors = null; - this._unmatchedSelectors = null; this._propertyInfos = {}; } else { // Update the CssPropertyInfo objects. @@ -468,7 +462,6 @@ CssLogic.prototype = { } this._matchedSelectors = []; - this._unmatchedSelectors = null; this._passId++; for (let i = 0; i < this._matchedRules.length; i++) { @@ -513,52 +506,6 @@ CssLogic.prototype = { return false; }, - /** - * Process the CssSelector object that do not match the highlighted elements, - * nor its parents. Your callback function is invoked for every such - * CssSelector object. You receive one argument: the CssSelector object. - * - * The list of unmatched selectors is cached. - * - * @param {function} aCallback the function you want to execute for each of - * the unmatched selectors. - * @param {object} aScope the scope you want for the callback function. aScope - * will be the this object when aCallback executes. - */ - processUnmatchedSelectors: function CL_processUnmatchedSelectors(aCallback, aScope) - { - if (this._unmatchedSelectors) { - if (aCallback) { - this._unmatchedSelectors.forEach(aCallback, aScope); - } - return; - } - - if (!this._matchedSelectors) { - this.processMatchedSelectors(); - } - - this._unmatchedSelectors = []; - - this.forEachSheet(function (aSheet) { - // We do not show unmatched selectors from system stylesheets - if (!aSheet.contentSheet || aSheet.disabled || !aSheet.mediaMatches) { - return; - } - - aSheet.forEachRule(function (aRule) { - aRule.selectors.forEach(function (aSelector) { - if (aSelector._matchId !== this._matchId) { - this._unmatchedSelectors.push(aSelector); - if (aCallback) { - aCallback.call(aScope, aSelector); - } - } - }, this); - }, this); - }, this); - }, - /** * Check if the highlighted element or it's parents have matched selectors. * @@ -667,95 +614,6 @@ CssLogic.prototype = { element.nodeType === Ci.nsIDOMNode.ELEMENT_NODE); }, - /** - * Check if the highlighted element or it's parents have unmatched selectors. - * - * Please note that this method is far slower than hasMatchedSelectors() - * because it needs to do a lot more checks in the DOM. - * - * @param {array} aProperties The list of properties you want to check if they - * have unmatched selectors or not. - * @return {object} An object that tells for each property if it has unmatched - * selectors or not. Object keys are property names and values are booleans. - */ - hasUnmatchedSelectors: function CL_hasUnmatchedSelectors(aProperties) - { - if (!this._matchedRules) { - this._buildMatchedRules(); - } - - let result = {}; - - this.forSomeSheets(function (aSheet) { - if (!aSheet.contentSheet || aSheet.disabled || !aSheet.mediaMatches) { - return false; - } - - return aSheet.forSomeRules(function (aRule) { - let unmatched = aRule._matchId !== this._matchId || - this._ruleHasUnmatchedSelector(aRule); - if (!unmatched) { - return false; - } - - aProperties = aProperties.filter(function(aProperty) { - if (!aRule.getPropertyValue(aProperty)) { - // Keep this property for the next rule. We need to find a rule - // which has the property. - return true; - } - - result[aProperty] = true; - - // We found a rule that has the current property while it does not - // match the current element. We can remove this property from the - // array. - return false; - }); - - return aProperties.length == 0; - }, this); - }, this); - - aProperties.forEach(function(aProperty) { result[aProperty] = false; }); - - return result; - }, - - /** - * Check if a CssRule has an unmatched selector for the highlighted element or - * its parents. - * - * @private - * @param {CssRule} aRule The rule you want to check if it has an unmatched - * selector. - * @return {boolean} True if the rule has an unmatched selector, false - * otherwise. - */ - _ruleHasUnmatchedSelector: function CL__ruleHasUnmatchedSelector(aRule) - { - if (!aRule._cssSheet && aRule.sourceElement) { - // CssRule wraps element.style, which never has unmatched selectors. - return false; - } - - let element = this.viewedElement; - let selectors = aRule.selectors; - - do { - selectors = selectors.filter(function(aSelector) { - return !element.mozMatchesSelector(aSelector); - }); - - if (selectors.length == 0) { - break; - } - } while ((element = element.parentNode) && - element.nodeType === Ci.nsIDOMNode.ELEMENT_NODE); - - return selectors.length > 0; - }, - /** * Tells if the given DOM CSS object matches the current view media. * @@ -1546,9 +1404,9 @@ CssSelector.prototype = { * A cache of information about the matched rules, selectors and values attached * to a CSS property, for the highlighted element. * - * The heart of the CssPropertyInfo object is the _findMatchedSelectors() and - * _findUnmatchedSelectors() methods. These are invoked when the PropertyView - * tries to access the .matchedSelectors and .unmatchedSelectors arrays. + * The heart of the CssPropertyInfo object is the _findMatchedSelectors() + * method. This are invoked when the PropertyView tries to access the + * .matchedSelectors array. * Results are cached, for later reuse. * * @param {CssLogic} aCssLogic Reference to the parent CssLogic instance @@ -1570,7 +1428,6 @@ function CssPropertyInfo(aCssLogic, aProperty) // counted. This includes rules that come from filtered stylesheets (those // that have sheetAllowed = false). this._matchedSelectors = null; - this._unmatchedSelectors = null; } CssPropertyInfo.prototype = { @@ -1613,23 +1470,6 @@ CssPropertyInfo.prototype = { return this._matchedRuleCount; }, - /** - * Retrieve the number of unmatched rules. - * - * @return {number} the number of rules that do not match the highlighted - * element or its parents. - */ - get unmatchedRuleCount() - { - if (!this._unmatchedSelectors) { - this._findUnmatchedSelectors(); - } else if (this.needRefilter) { - this._refilterSelectors(); - } - - return this._unmatchedRuleCount; - }, - /** * Retrieve the array holding CssSelectorInfo objects for each of the matched * selectors, from each of the matched rules. Only selectors coming from @@ -1649,25 +1489,6 @@ CssPropertyInfo.prototype = { return this._matchedSelectors; }, - /** - * Retrieve the array holding CssSelectorInfo objects for each of the - * unmatched selectors, from each of the unmatched rules. Only selectors - * coming from allowed stylesheets are included in the array. - * - * @return {array} the list of CssSelectorInfo objects of selectors that do - * not match the highlighted element or its parents. - */ - get unmatchedSelectors() - { - if (!this._unmatchedSelectors) { - this._findUnmatchedSelectors(); - } else if (this.needRefilter) { - this._refilterSelectors(); - } - - return this._unmatchedSelectors; - }, - /** * Find the selectors that match the highlighted element and its parents. * Uses CssLogic.processMatchedSelectors() to find the matched selectors, @@ -1726,53 +1547,8 @@ CssPropertyInfo.prototype = { }, /** - * Find the selectors that do not match the highlighted element and its - * parents. - * @private - */ - _findUnmatchedSelectors: function CssPropertyInfo_findUnmatchedSelectors() - { - this._unmatchedSelectors = []; - this._unmatchedRuleCount = 0; - this.needRefilter = false; - this._cssLogic._passId++; - - this._cssLogic.processUnmatchedSelectors(this._processUnmatchedSelector, - this); - - // Sort the selectors by specificity. - this._unmatchedSelectors.sort(function(aSelectorInfo1, aSelectorInfo2) { - return aSelectorInfo1.compareTo(aSelectorInfo2); - }); - }, - - /** - * Process an unmatched CssSelector object. Note that in order to avoid - * information overload we DO NOT show unmatched system rules. - * - * @private - * @param {CssSelector} aSelector the unmatched CssSelector object. - */ - _processUnmatchedSelector: function CPI_processUnmatchedSelector(aSelector) - { - let cssRule = aSelector._cssRule; - let value = cssRule.getPropertyValue(this.property); - if (value) { - let selectorInfo = new CssSelectorInfo(aSelector, this.property, value, - CssLogic.STATUS.UNMATCHED); - this._unmatchedSelectors.push(selectorInfo); - if (this._cssLogic._passId != cssRule._passId) { - if (cssRule.sheetAllowed) { - this._unmatchedRuleCount++; - } - cssRule._passId = this._cssLogic._passId; - } - } - }, - - /** - * Refilter the matched and unmatched selectors arrays when the - * CssLogic.sourceFilter changes. This allows for quick filter changes. + * Refilter the matched selectors array when the CssLogic.sourceFilter + * changes. This allows for quick filter changes. * @private */ _refilterSelectors: function CssPropertyInfo_refilterSelectors() @@ -1795,12 +1571,6 @@ CssPropertyInfo.prototype = { this._matchedRuleCount = ruleCount; } - if (this._unmatchedSelectors) { - ruleCount = 0; - this._unmatchedSelectors.forEach(iterator); - this._unmatchedRuleCount = ruleCount; - } - this.needRefilter = false; }, @@ -1813,10 +1583,10 @@ CssPropertyInfo.prototype = { /** * A class that holds information about a given CssSelector object. * - * Instances of this class are given to CssHtmlTree in the arrays of matched and - * unmatched selectors. Each such object represents a displayable row in the - * PropertyView objects. The information given by this object blends data coming - * from the CssSheet, CssRule and from the CssSelector that own this object. + * Instances of this class are given to CssHtmlTree in the array of matched + * selectors. Each such object represents a displayable row in the PropertyView + * objects. The information given by this object blends data coming from the + * CssSheet, CssRule and from the CssSelector that own this object. * * @param {CssSelector} aSelector The CssSelector object for which to present information. * @param {string} aProperty The property for which information should be retrieved. diff --git a/browser/devtools/styleinspector/test/browser_bug683672.html b/browser/devtools/styleinspector/test/browser_bug683672.html index 78f963550182..8fe007409cb3 100644 --- a/browser/devtools/styleinspector/test/browser_bug683672.html +++ b/browser/devtools/styleinspector/test/browser_bug683672.html @@ -7,10 +7,6 @@ color: #000; } - .unmatched1, .unmatched2, .unmatched3, .unmatched4, .unmatched5, .unmatched6, .unmatched7 { - color: #f00; - } - div { position: absolute; top: 40px; @@ -29,4 +25,4 @@
- \ No newline at end of file + diff --git a/browser/devtools/styleinspector/test/browser_bug683672.js b/browser/devtools/styleinspector/test/browser_bug683672.js index bd9064df0deb..0409c0f3b913 100644 --- a/browser/devtools/styleinspector/test/browser_bug683672.js +++ b/browser/devtools/styleinspector/test/browser_bug683672.js @@ -50,7 +50,6 @@ function selectNode(aInspector) function runTests() { testMatchedSelectors(); - //testUnmatchedSelectors(); info("finishing up"); finishUp(); @@ -73,28 +72,6 @@ function testMatchedSelectors() "hasMatchedSelectors returns true"); } -function testUnmatchedSelectors() -{ - info("checking selector counts, unmatched rules and titles"); - let body = content.document.body; - ok(body, "captain, we have a body"); - - info("selecting content.document.body"); - inspector.selection.setNode(body); - - is(body, computedView.viewedElement, - "style inspector node matches the selected node"); - - let propertyView = new PropertyView(computedView, "color"); - let numUnmatchedSelectors = propertyView.propertyInfo.unmatchedSelectors.length; - - is(numUnmatchedSelectors, 13, - "CssLogic returns the correct number of unmatched selectors for body"); - - is(propertyView.hasUnmatchedSelectors, true, - "hasUnmatchedSelectors returns true"); -} - function finishUp() { doc = inspector = div = computedView = null; diff --git a/browser/devtools/styleinspector/test/browser_styleinspector.js b/browser/devtools/styleinspector/test/browser_styleinspector.js index ea8e01203ee4..78c67ef6a62a 100644 --- a/browser/devtools/styleinspector/test/browser_styleinspector.js +++ b/browser/devtools/styleinspector/test/browser_styleinspector.js @@ -66,7 +66,6 @@ function SI_CheckProperty() let cssLogic = computedView.cssLogic; let propertyInfo = cssLogic.getPropertyInfo("color"); ok(propertyInfo.matchedRuleCount > 0, "color property has matching rules"); - //ok(propertyInfo.unmatchedRuleCount > 0, "color property has unmatched rules"); } function finishUp() diff --git a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd index e28e4e07b416..ad305e5b9809 100644 --- a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd +++ b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd @@ -27,20 +27,13 @@ - displayed. --> - - - - + - diff --git a/browser/locales/en-US/chrome/browser/devtools/styleinspector.properties b/browser/locales/en-US/chrome/browser/devtools/styleinspector.properties index 5f7f1861025d..4c10869fc283 100644 --- a/browser/locales/en-US/chrome/browser/devtools/styleinspector.properties +++ b/browser/locales/en-US/chrome/browser/devtools/styleinspector.properties @@ -21,7 +21,6 @@ panelTitle=Style Inspector rule.status.BEST=Best Match rule.status.MATCHED=Matched rule.status.PARENT_MATCH=Parent Match -rule.status.UNMATCHED=Unmatched # LOCALIZATION NOTE (rule.sourceElement, rule.sourceInline): For each # style property the panel shows the rules which hold that specific property. From ab29b54ccf02f77590081fcac86dae581e0b53d4 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Tue, 5 Feb 2013 16:45:01 +0000 Subject: [PATCH 07/15] Bug 789785 - [inspector] Change wording of "Only user styles" checkbox --- .../devtools/styleinspector/CssHtmlTree.jsm | 29 ++++++++++--------- .../devtools/styleinspector/csshtmltree.xul | 7 +++-- ...styleinspector_bug_672744_search_filter.js | 4 +-- ...tyleinspector_bug_672746_default_styles.js | 4 +-- .../browser/devtools/styleinspector.dtd | 4 +-- .../gnomestripe/devtools/csshtmltree.css | 2 +- .../themes/pinstripe/devtools/csshtmltree.css | 2 +- .../themes/winstripe/devtools/csshtmltree.css | 2 +- 8 files changed, 28 insertions(+), 26 deletions(-) diff --git a/browser/devtools/styleinspector/CssHtmlTree.jsm b/browser/devtools/styleinspector/CssHtmlTree.jsm index 678a586eae91..f3e91fd7148e 100644 --- a/browser/devtools/styleinspector/CssHtmlTree.jsm +++ b/browser/devtools/styleinspector/CssHtmlTree.jsm @@ -227,8 +227,8 @@ CssHtmlTree.prototype = { // The search filter searchField: null, - // Reference to the "Only user Styles" checkbox. - onlyUserStylesCheckbox: null, + // Reference to the "Include browser styles" checkbox. + includeBrowserStylesCheckbox: null, // Holds the ID of the panelRefresh timeout. _panelRefreshTimeout: null, @@ -239,9 +239,9 @@ CssHtmlTree.prototype = { // Number of visible properties numVisibleProperties: 0, - get showOnlyUserStyles() + get includeBrowserStyles() { - return this.onlyUserStylesCheckbox.checked; + return this.includeBrowserStylesCheckbox.checked; }, /** @@ -369,28 +369,29 @@ CssHtmlTree.prototype = { }, /** - * The change event handler for the onlyUserStyles checkbox. + * The change event handler for the includeBrowserStyles checkbox. * * @param {Event} aEvent the DOM Event object. */ - onlyUserStylesChanged: function CssHtmltree_onlyUserStylesChanged(aEvent) + includeBrowserStylesChanged: + function CssHtmltree_includeBrowserStylesChanged(aEvent) { this.refreshSourceFilter(); this.refreshPanel(); }, /** - * When onlyUserStyles.checked is true we only display properties that have - * matched selectors and have been included by the document or one of the + * When includeBrowserStyles.checked is false we only display properties that + * have matched selectors and have been included by the document or one of the * document's stylesheets. If .checked is false we display all properties * including those that come from UA stylesheets. */ refreshSourceFilter: function CssHtmlTree_setSourceFilter() { this._matchedProperties = null; - this.cssLogic.sourceFilter = this.showOnlyUserStyles ? - CssLogic.FILTER.ALL : - CssLogic.FILTER.UA; + this.cssLogic.sourceFilter = this.includeBrowserStyles ? + CssLogic.FILTER.UA : + CssLogic.FILTER.ALL; }, /** @@ -651,8 +652,8 @@ CssHtmlTree.prototype = { delete this.viewedElement; // Remove event listeners - this.onlyUserStylesCheckbox.removeEventListener("command", - this.onlyUserStylesChanged); + this.includeBrowserStylesCheckbox.removeEventListener("command", + this.includeBrowserStylesChanged); this.searchField.removeEventListener("command", this.filterChanged); // Cancel tree construction @@ -784,7 +785,7 @@ PropertyView.prototype = { */ get visible() { - if (this.tree.showOnlyUserStyles && !this.hasMatchedSelectors) { + if (!this.tree.includeBrowserStyles && !this.hasMatchedSelectors) { return false; } diff --git a/browser/devtools/styleinspector/csshtmltree.xul b/browser/devtools/styleinspector/csshtmltree.xul index 084f5e0f9d0e..733e4ebb4c13 100644 --- a/browser/devtools/styleinspector/csshtmltree.xul +++ b/browser/devtools/styleinspector/csshtmltree.xul @@ -67,9 +67,10 @@ To visually debug the templates without running firefox, alter the display:none -->
- + diff --git a/browser/devtools/styleinspector/test/browser_styleinspector_bug_672744_search_filter.js b/browser/devtools/styleinspector/test/browser_styleinspector_bug_672744_search_filter.js index a7c48a0befa6..64de25d7dff9 100644 --- a/browser/devtools/styleinspector/test/browser_styleinspector_bug_672744_search_filter.js +++ b/browser/devtools/styleinspector/test/browser_styleinspector_bug_672744_search_filter.js @@ -54,10 +54,10 @@ function SI_toggleDefaultStyles() { Services.obs.removeObserver(SI_toggleDefaultStyles, "StyleInspector-populated", false); - info("clearing \"only user styles\" checkbox"); + info("checking \"Browser styles\" checkbox"); let doc = computedView.styleDocument; - let checkbox = doc.querySelector(".onlyuserstyles"); + let checkbox = doc.querySelector(".includebrowserstyles"); Services.obs.addObserver(SI_AddFilterText, "StyleInspector-populated", false); checkbox.click(); } diff --git a/browser/devtools/styleinspector/test/browser_styleinspector_bug_672746_default_styles.js b/browser/devtools/styleinspector/test/browser_styleinspector_bug_672746_default_styles.js index 059134f19f07..ae122ee61787 100644 --- a/browser/devtools/styleinspector/test/browser_styleinspector_bug_672746_default_styles.js +++ b/browser/devtools/styleinspector/test/browser_styleinspector_bug_672746_default_styles.js @@ -2,7 +2,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -// Tests that the checkbox to show only user styles works properly. +// Tests that the checkbox to include browser styles works properly. let doc; let inspector; @@ -65,7 +65,7 @@ function SI_toggleDefaultStyles() { // Click on the checkbox. let doc = computedView.styleDocument; - let checkbox = doc.querySelector(".onlyuserstyles"); + let checkbox = doc.querySelector(".includebrowserstyles"); Services.obs.addObserver(SI_checkDefaultStyles, "StyleInspector-populated", false); checkbox.click(); diff --git a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd index ad305e5b9809..b5aec6945a70 100644 --- a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd +++ b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd @@ -8,10 +8,10 @@ - A good criteria is the language in which you'd find the best - documentation on web development on the web. --> - - + diff --git a/browser/themes/gnomestripe/devtools/csshtmltree.css b/browser/themes/gnomestripe/devtools/csshtmltree.css index fedb2fc44925..d1de70c8c850 100644 --- a/browser/themes/gnomestripe/devtools/csshtmltree.css +++ b/browser/themes/gnomestripe/devtools/csshtmltree.css @@ -124,7 +124,7 @@ background-color: -moz-dialog; } -.onlyuserstyles { +.includebrowserstyles { cursor: pointer; font-size: 11px; } diff --git a/browser/themes/pinstripe/devtools/csshtmltree.css b/browser/themes/pinstripe/devtools/csshtmltree.css index c9f7ca378ba5..8877404d913e 100644 --- a/browser/themes/pinstripe/devtools/csshtmltree.css +++ b/browser/themes/pinstripe/devtools/csshtmltree.css @@ -126,7 +126,7 @@ background-color: -moz-dialog; } -.onlyuserstyles { +.includebrowserstyles { cursor: pointer; font-size: 11px; } diff --git a/browser/themes/winstripe/devtools/csshtmltree.css b/browser/themes/winstripe/devtools/csshtmltree.css index 05c2196cf6c9..7998ca19c49c 100644 --- a/browser/themes/winstripe/devtools/csshtmltree.css +++ b/browser/themes/winstripe/devtools/csshtmltree.css @@ -126,7 +126,7 @@ background-color: -moz-dialog; } -.onlyuserstyles { +.includebrowserstyles { cursor: pointer; font-size: 11px; } From 3052721048e93bb48eafeba4b6cb699c8a97ef35 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 6 Feb 2013 15:55:10 +0000 Subject: [PATCH 08/15] Bug 817294 - Intermittent browser_styleeditor_loading.js | content document is still loading - Didn't expect complete, but got it | style editor root element has 'loading' class name | new style sheet button is disabled | import button is disabled --- browser/devtools/styleeditor/StyleEditor.jsm | 5 +- .../styleeditor/StyleEditorChrome.jsm | 133 ++++++++---------- .../devtools/styleeditor/StyleEditorPanel.jsm | 16 ++- .../devtools/styleeditor/StyleEditorUtil.jsm | 2 +- browser/devtools/styleeditor/test/Makefile.in | 3 +- .../test/browser_styleeditor_import.js | 5 +- .../test/browser_styleeditor_init.js | 15 +- .../test/browser_styleeditor_loading.js | 51 ++----- .../test/browser_styleeditor_new.js | 5 +- ...browser_styleeditor_private_perwindowpb.js | 8 +- .../test/browser_styleeditor_sv_keynav.js | 7 +- .../test/browser_styleeditor_sv_resize.js | 19 +-- .../devtools/styleeditor/test/longload.html | 28 ++++ 13 files changed, 129 insertions(+), 168 deletions(-) create mode 100644 browser/devtools/styleeditor/test/longload.html diff --git a/browser/devtools/styleeditor/StyleEditor.jsm b/browser/devtools/styleeditor/StyleEditor.jsm index 8052b14d72ac..db8aaabe56c1 100644 --- a/browser/devtools/styleeditor/StyleEditor.jsm +++ b/browser/devtools/styleeditor/StyleEditor.jsm @@ -130,7 +130,7 @@ StyleEditor.prototype = { { let document = this.contentDocument; if (this._styleSheetIndex == -1) { - for (let i = 0; i < document.styleSheets.length; ++i) { + for (let i = 0; i < document.styleSheets.length; i++) { if (document.styleSheets[i] == this.styleSheet) { this._styleSheetIndex = i; break; @@ -1004,8 +1004,9 @@ StyleEditor.prototype = { // copy the list of listeners to allow adding/removing listeners in handlers let listeners = this._actionListeners.concat(); + // trigger all listeners that have this action handler - for (let i = 0; i < listeners.length; ++i) { + for (let i = 0; i < listeners.length; i++) { let listener = listeners[i]; let actionHandler = listener["on" + aName]; if (actionHandler) { diff --git a/browser/devtools/styleeditor/StyleEditorChrome.jsm b/browser/devtools/styleeditor/StyleEditorChrome.jsm index a8da29eb837a..acc75ffeb384 100644 --- a/browser/devtools/styleeditor/StyleEditorChrome.jsm +++ b/browser/devtools/styleeditor/StyleEditorChrome.jsm @@ -16,6 +16,7 @@ Cu.import("resource://gre/modules/PluralForm.jsm"); Cu.import("resource:///modules/devtools/StyleEditor.jsm"); Cu.import("resource:///modules/devtools/StyleEditorUtil.jsm"); Cu.import("resource:///modules/devtools/SplitView.jsm"); +Cu.import("resource://gre/modules/commonjs/promise/core.js"); const STYLE_EDITOR_TEMPLATE = "stylesheet"; @@ -43,34 +44,43 @@ this.StyleEditorChrome = function StyleEditorChrome(aRoot, aContentWindow) this._editors = []; this._listeners = []; // @see addChromeListener + // Store the content window so that we can call the real contentWindow setter + // in the open method. + this._contentWindowTemp = aContentWindow; + this._contentWindow = null; - this._isContentAttached = false; - - let initializeUI = function (aEvent) { - if (aEvent) { - this._window.removeEventListener("load", initializeUI, false); - } - - let viewRoot = this._root.parentNode.querySelector(".splitview-root"); - this._view = new SplitView(viewRoot); - - this._setupChrome(); - - // attach to the content window - this.contentWindow = aContentWindow; - this._contentWindowID = null; - }.bind(this); - - if (this._document.readyState == "complete") { - initializeUI(); - } else { - this._window.addEventListener("load", initializeUI, false); - } } StyleEditorChrome.prototype = { _styleSheetToSelect: null, + open: function() { + let deferred = Promise.defer(); + let initializeUI = function (aEvent) { + if (aEvent) { + this._window.removeEventListener("load", initializeUI, false); + } + let viewRoot = this._root.parentNode.querySelector(".splitview-root"); + this._view = new SplitView(viewRoot); + this._setupChrome(); + + // We need to juggle arount the contentWindow items because we need to + // trigger the setter at the appropriate time. + this.contentWindow = this._contentWindowTemp; // calls setter + this._contentWindowTemp = null; + + deferred.resolve(); + }.bind(this); + + if (this._document.readyState == "complete") { + initializeUI(); + } else { + this._window.addEventListener("load", initializeUI, false); + } + + return deferred.promise; + }, + /** * Retrieve the content window attached to this chrome. * @@ -150,15 +160,6 @@ StyleEditorChrome.prototype = { return this._contentWindow ? this._contentWindow.document : null; }, - /** - * Retrieve whether the content has been attached and StyleEditor instances - * exist for all of its stylesheets. - * - * @return boolean - * @see addChromeListener - */ - get isContentAttached() this._isContentAttached, - /** * Retrieve an array with the StyleEditor instance for each live style sheet, * ordered by style sheet index. @@ -180,14 +181,6 @@ StyleEditorChrome.prototype = { * Add a listener for StyleEditorChrome events. * * The listener implements IStyleEditorChromeListener := { - * onContentAttach: Called when a content window has been attached. - * All editors are instantiated, though they might - * not be loaded yet. - * Arguments: (StyleEditorChrome aChrome) - * @see contentWindow - * @see StyleEditor.isLoaded - * @see StyleEditor.addActionListener - * * onContentDetach: Called when the content window has been detached. * Arguments: (StyleEditorChrome aChrome) * @see contentWindow @@ -287,7 +280,7 @@ StyleEditorChrome.prototype = { // (re)enable UI let matches = this._root.querySelectorAll("toolbarbutton,input,select"); - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { matches[i].removeAttribute("disabled"); } }, @@ -305,7 +298,7 @@ StyleEditorChrome.prototype = { this._document.title = _("chromeWindowTitle", document.title || document.location.href); - for (let i = 0; i < document.styleSheets.length; ++i) { + for (let i = 0; i < document.styleSheets.length; i++) { let styleSheet = document.styleSheets[i]; let editor = new StyleEditor(document, styleSheet); @@ -313,8 +306,6 @@ StyleEditorChrome.prototype = { this._editors.push(editor); } - this._triggerChromeListeners("ContentAttach"); - // Queue editors loading so that ContentAttach is consistently triggered // right after all editor instances are available (this.editors) but are // NOT loaded/ready yet. This also helps responsivity during loading when @@ -353,43 +344,36 @@ StyleEditorChrome.prototype = { let select = function DEC_select(aEditor) { let sheet = this._styleSheetToSelect.sheet; - let line = this._styleSheetToSelect.line; - let col = this._styleSheetToSelect.col; - let summary = sheet ? this.getSummaryElementForEditor(aEditor) - : this._view.getSummaryElementByOrdinal(0); + let line = this._styleSheetToSelect.line || 1; + let col = this._styleSheetToSelect.col || 1; - if (line) { - col = col || 1; - - if (!aEditor.sourceEditor) { - // If a line or column was specified we move the caret appropriately. - let self = this; - aEditor.addActionListener({ - onAttach: function SEC_selectSheet_onAttach() - { - aEditor.removeActionListener(this); - self.selectedStyleSheetIndex = aEditor.styleSheetIndex; - aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); - - let newSheet = self._styleSheetToSelect.sheet; - let newLine = self._styleSheetToSelect.line; - let newCol = self._styleSheetToSelect.col; - self._styleSheetToSelect = null; - if (newSheet != sheet) { - self._window.setTimeout(self.selectStyleSheet.bind(self, newSheet, newLine, newCol), 0); - } - } - }); - } else { - // If a line or column was specified we move the caret appropriately. + if (!aEditor.sourceEditor) { + let onAttach = function SEC_selectSheet_onAttach() { + aEditor.removeActionListener(this); + this.selectedStyleSheetIndex = aEditor.styleSheetIndex; aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); + + let newSheet = this._styleSheetToSelect.sheet; + let newLine = this._styleSheetToSelect.line; + let newCol = this._styleSheetToSelect.col; this._styleSheetToSelect = null; - } + if (newSheet != sheet) { + this.selectStyleSheet.bind(this, newSheet, newLine, newCol); + } + }.bind(this); + + aEditor.addActionListener({ + onAttach: onAttach + }); } else { + // If a line or column was specified we move the caret appropriately. + aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); this._styleSheetToSelect = null; } - this._view.activeSummary = summary; + let summary = sheet ? this.getSummaryElementForEditor(aEditor) + : this._view.getSummaryElementByOrdinal(0); + this._view.activeSummary = summary; this.selectedStyleSheetIndex = aEditor.styleSheetIndex; }.bind(this); @@ -404,6 +388,7 @@ StyleEditorChrome.prototype = { if ((sheet && aEditor.styleSheet == sheet) || (aEditor.styleSheetIndex == 0 && sheet == null)) { aChrome.removeChromeListener(this); + aEditor.addActionListener(self); select(aEditor); } } @@ -430,7 +415,7 @@ StyleEditorChrome.prototype = { _disableChrome: function SEC__disableChrome() { let matches = this._root.querySelectorAll("button,toolbarbutton,textbox"); - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { matches[i].setAttribute("disabled", "disabled"); } diff --git a/browser/devtools/styleeditor/StyleEditorPanel.jsm b/browser/devtools/styleeditor/StyleEditorPanel.jsm index 7122a495fe85..f84b856d1412 100644 --- a/browser/devtools/styleeditor/StyleEditorPanel.jsm +++ b/browser/devtools/styleeditor/StyleEditorPanel.jsm @@ -39,10 +39,14 @@ StyleEditorPanel.prototype = { */ open: function StyleEditor_open() { let contentWin = this._toolbox.target.window; - this.setPage(contentWin); - this.isReady = true; + let deferred = Promise.defer(); - return Promise.resolve(this); + this.setPage(contentWin).then(function() { + this.isReady = true; + deferred.resolve(this); + }.bind(this)); + + return deferred.promise; }, /** @@ -66,12 +70,16 @@ StyleEditorPanel.prototype = { setPage: function StyleEditor_setPage(contentWindow) { if (this._panelWin.styleEditorChrome) { this._panelWin.styleEditorChrome.contentWindow = contentWindow; + this.selectStyleSheet(null, null, null); } else { let chromeRoot = this._panelDoc.getElementById("style-editor-chrome"); let chrome = new StyleEditorChrome(chromeRoot, contentWindow); + let promise = chrome.open(); + this._panelWin.styleEditorChrome = chrome; + this.selectStyleSheet(null, null, null); + return promise; } - this.selectStyleSheet(null, null, null); }, /** diff --git a/browser/devtools/styleeditor/StyleEditorUtil.jsm b/browser/devtools/styleeditor/StyleEditorUtil.jsm index 7ef4a10c6fec..f083a47c8f3d 100644 --- a/browser/devtools/styleeditor/StyleEditorUtil.jsm +++ b/browser/devtools/styleeditor/StyleEditorUtil.jsm @@ -152,7 +152,7 @@ this.wire = function wire(aRoot, aSelectorOrElement, aDescriptor) aDescriptor = {events: {click: aDescriptor}}; } - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { let element = matches[i]; forEach(aDescriptor.events, function (aName, aHandler) { element.addEventListener(aName, aHandler, false); diff --git a/browser/devtools/styleeditor/test/Makefile.in b/browser/devtools/styleeditor/test/Makefile.in index f458d19c2a6b..addbb245203f 100644 --- a/browser/devtools/styleeditor/test/Makefile.in +++ b/browser/devtools/styleeditor/test/Makefile.in @@ -18,7 +18,7 @@ _BROWSER_TEST_FILES = \ browser_styleeditor_cmd_edit.html \ browser_styleeditor_import.js \ browser_styleeditor_init.js \ - $(filter disabled-temporarily--bug-817294, browser_styleeditor_loading.js) \ + browser_styleeditor_loading.js \ browser_styleeditor_new.js \ browser_styleeditor_passedinsheet.js \ browser_styleeditor_pretty.js \ @@ -32,6 +32,7 @@ _BROWSER_TEST_FILES = \ four.html \ head.js \ helpers.js \ + longload.html \ media.html \ media-small.css \ minified.html \ diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_import.js b/browser/devtools/styleeditor/test/browser_styleeditor_import.js index f8d15d8a463a..cd6db75ace81 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_import.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_import.js @@ -19,12 +19,9 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_init.js b/browser/devtools/styleeditor/test/browser_styleeditor_init.js index 7835c7c15ba2..c106e3db9bc1 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_init.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_init.js @@ -9,23 +9,17 @@ function test() { waitForExplicitFinish(); - addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { + launchStyleEditorChrome(function(aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); - content.location = TESTCASE_URI; } -let gContentAttachHandled = false; function run(aChrome) { - gContentAttachHandled = true; is(aChrome.contentWindow.document.readyState, "complete", "content document is complete"); @@ -42,11 +36,6 @@ function run(aChrome) let gEditorAddedCount = 0; function testEditorAdded(aChrome, aEditor) { - if (!gEditorAddedCount) { - is(gContentAttachHandled, true, - "ContentAttach event triggered before EditorAdded"); - } - if (aEditor.styleSheetIndex == 0) { gEditorAddedCount++; testFirstStyleSheetEditor(aChrome, aEditor); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js index 07331c6e092c..c422785fc84d 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js @@ -2,7 +2,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -const TESTCASE_URI = TEST_BASE + "simple.html"; +const TESTCASE_URI = TEST_BASE + "longload.html"; function test() @@ -13,52 +13,25 @@ function test() // launch Style Editor right when the tab is created (before load) // this checks that the Style Editor still launches correctly when it is opened - // *while* the page is still loading + // *while* the page is still loading. The Style Editor should not signal that + // it is loaded until the accompanying content page is loaded. launchStyleEditorChrome(function (aChrome) { content.location = TESTCASE_URI; - executeSoon(function() { - isnot(gBrowser.selectedBrowser.contentWindow.document.readyState, "complete", - "content document is still loading"); + is(aChrome.contentWindow.document.readyState, "complete", + "content document is complete"); let root = gChromeWindow.document.querySelector(".splitview-root"); - ok(root.classList.contains("loading"), - "style editor root element has 'loading' class name"); + ok(!root.classList.contains("loading"), + "style editor root element does not have 'loading' class name anymore"); let button = gChromeWindow.document.querySelector(".style-editor-newButton"); - ok(button.hasAttribute("disabled"), - "new style sheet button is disabled"); + ok(!button.hasAttribute("disabled"), + "new style sheet button is enabled"); button = gChromeWindow.document.querySelector(".style-editor-importButton"); - ok(button.hasAttribute("disabled"), - "import button is disabled"); + ok(!button.hasAttribute("disabled"), + "import button is enabled"); - if (!aChrome.isContentAttached) { - aChrome.addChromeListener({ - onContentAttach: run - }); - } else { - run(aChrome); - } - }); + finish(); }); } - -function run(aChrome) -{ - is(aChrome.contentWindow.document.readyState, "complete", - "content document is complete"); - - let root = gChromeWindow.document.querySelector(".splitview-root"); - ok(!root.classList.contains("loading"), - "style editor root element does not have 'loading' class name anymore"); - - let button = gChromeWindow.document.querySelector(".style-editor-newButton"); - ok(!button.hasAttribute("disabled"), - "new style sheet button is enabled"); - - button = gChromeWindow.document.querySelector(".style-editor-importButton"); - ok(!button.hasAttribute("disabled"), - "import button is enabled"); - - finish(); -} diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_new.js b/browser/devtools/styleeditor/test/browser_styleeditor_new.js index f8de9da49508..3d05336123c1 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_new.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_new.js @@ -13,12 +13,9 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js index 58ac1796b727..13210fad6bcb 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js @@ -19,13 +19,7 @@ function test() { aWindow.gBrowser.selectedBrowser.removeEventListener("load", onLoad, true); cache.evictEntries(Ci.nsICache.STORE_ANYWHERE); launchStyleEditorChromeFromWindow(aWindow, function(aChrome) { - if (aChrome.isContentAttached) { - onEditorAdded(aChrome, aChrome.editors[0]); - } else { - aChrome.addChromeListener({ - onEditorAdded: onEditorAdded - }); - } + onEditorAdded(aChrome, aChrome.editors[0]); }); }, true); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js index e734f7d0237a..151df51e64db 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js @@ -10,12 +10,7 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - aChrome.addChromeListener({ - onContentAttach: run - }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js index 4c2766363b77..37f2e7cf55b3 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js @@ -12,15 +12,8 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - if (aChrome.isContentAttached) { - run(aChrome); - } else { - aChrome.addChromeListener({ - onContentAttach: run - }); - } + run(aChrome); }); - content.location = TESTCASE_URI; } @@ -31,13 +24,13 @@ function run(aChrome) aChrome.editors[0].addActionListener({ onAttach: function onEditorAttached(aEditor) { - let originalSourceEditor = aEditor.sourceEditor; - aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved - - // queue a resize to inverse aspect ratio - // this will trigger a detach and reattach (to workaround bug 254144) executeSoon(function () { waitForFocus(function () { + // queue a resize to inverse aspect ratio + // this will trigger a detach and reattach (to workaround bug 254144) + let originalSourceEditor = aEditor.sourceEditor; + aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved + gOriginalWidth = gChromeWindow.outerWidth; gOriginalHeight = gChromeWindow.outerHeight; gChromeWindow.resizeTo(120, 480); diff --git a/browser/devtools/styleeditor/test/longload.html b/browser/devtools/styleeditor/test/longload.html new file mode 100644 index 000000000000..f4c147589754 --- /dev/null +++ b/browser/devtools/styleeditor/test/longload.html @@ -0,0 +1,28 @@ + + + + Long load + + + + + Time passes: + + + From 50668ccc45cb0ceb034d77e9b0a0cc36732c2193 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 6 Feb 2013 17:40:30 +0000 Subject: [PATCH 09/15] Backed out changeset 9a0b4b742b67 - Bug 817294 orange --- browser/devtools/styleeditor/StyleEditor.jsm | 5 +- .../styleeditor/StyleEditorChrome.jsm | 133 ++++++++++-------- .../devtools/styleeditor/StyleEditorPanel.jsm | 16 +-- .../devtools/styleeditor/StyleEditorUtil.jsm | 2 +- browser/devtools/styleeditor/test/Makefile.in | 3 +- .../test/browser_styleeditor_import.js | 5 +- .../test/browser_styleeditor_init.js | 15 +- .../test/browser_styleeditor_loading.js | 51 +++++-- .../test/browser_styleeditor_new.js | 5 +- ...browser_styleeditor_private_perwindowpb.js | 8 +- .../test/browser_styleeditor_sv_keynav.js | 7 +- .../test/browser_styleeditor_sv_resize.js | 19 ++- .../devtools/styleeditor/test/longload.html | 28 ---- 13 files changed, 168 insertions(+), 129 deletions(-) delete mode 100644 browser/devtools/styleeditor/test/longload.html diff --git a/browser/devtools/styleeditor/StyleEditor.jsm b/browser/devtools/styleeditor/StyleEditor.jsm index db8aaabe56c1..8052b14d72ac 100644 --- a/browser/devtools/styleeditor/StyleEditor.jsm +++ b/browser/devtools/styleeditor/StyleEditor.jsm @@ -130,7 +130,7 @@ StyleEditor.prototype = { { let document = this.contentDocument; if (this._styleSheetIndex == -1) { - for (let i = 0; i < document.styleSheets.length; i++) { + for (let i = 0; i < document.styleSheets.length; ++i) { if (document.styleSheets[i] == this.styleSheet) { this._styleSheetIndex = i; break; @@ -1004,9 +1004,8 @@ StyleEditor.prototype = { // copy the list of listeners to allow adding/removing listeners in handlers let listeners = this._actionListeners.concat(); - // trigger all listeners that have this action handler - for (let i = 0; i < listeners.length; i++) { + for (let i = 0; i < listeners.length; ++i) { let listener = listeners[i]; let actionHandler = listener["on" + aName]; if (actionHandler) { diff --git a/browser/devtools/styleeditor/StyleEditorChrome.jsm b/browser/devtools/styleeditor/StyleEditorChrome.jsm index acc75ffeb384..a8da29eb837a 100644 --- a/browser/devtools/styleeditor/StyleEditorChrome.jsm +++ b/browser/devtools/styleeditor/StyleEditorChrome.jsm @@ -16,7 +16,6 @@ Cu.import("resource://gre/modules/PluralForm.jsm"); Cu.import("resource:///modules/devtools/StyleEditor.jsm"); Cu.import("resource:///modules/devtools/StyleEditorUtil.jsm"); Cu.import("resource:///modules/devtools/SplitView.jsm"); -Cu.import("resource://gre/modules/commonjs/promise/core.js"); const STYLE_EDITOR_TEMPLATE = "stylesheet"; @@ -44,43 +43,34 @@ this.StyleEditorChrome = function StyleEditorChrome(aRoot, aContentWindow) this._editors = []; this._listeners = []; // @see addChromeListener - // Store the content window so that we can call the real contentWindow setter - // in the open method. - this._contentWindowTemp = aContentWindow; - this._contentWindow = null; + this._isContentAttached = false; + + let initializeUI = function (aEvent) { + if (aEvent) { + this._window.removeEventListener("load", initializeUI, false); + } + + let viewRoot = this._root.parentNode.querySelector(".splitview-root"); + this._view = new SplitView(viewRoot); + + this._setupChrome(); + + // attach to the content window + this.contentWindow = aContentWindow; + this._contentWindowID = null; + }.bind(this); + + if (this._document.readyState == "complete") { + initializeUI(); + } else { + this._window.addEventListener("load", initializeUI, false); + } } StyleEditorChrome.prototype = { _styleSheetToSelect: null, - open: function() { - let deferred = Promise.defer(); - let initializeUI = function (aEvent) { - if (aEvent) { - this._window.removeEventListener("load", initializeUI, false); - } - let viewRoot = this._root.parentNode.querySelector(".splitview-root"); - this._view = new SplitView(viewRoot); - this._setupChrome(); - - // We need to juggle arount the contentWindow items because we need to - // trigger the setter at the appropriate time. - this.contentWindow = this._contentWindowTemp; // calls setter - this._contentWindowTemp = null; - - deferred.resolve(); - }.bind(this); - - if (this._document.readyState == "complete") { - initializeUI(); - } else { - this._window.addEventListener("load", initializeUI, false); - } - - return deferred.promise; - }, - /** * Retrieve the content window attached to this chrome. * @@ -160,6 +150,15 @@ StyleEditorChrome.prototype = { return this._contentWindow ? this._contentWindow.document : null; }, + /** + * Retrieve whether the content has been attached and StyleEditor instances + * exist for all of its stylesheets. + * + * @return boolean + * @see addChromeListener + */ + get isContentAttached() this._isContentAttached, + /** * Retrieve an array with the StyleEditor instance for each live style sheet, * ordered by style sheet index. @@ -181,6 +180,14 @@ StyleEditorChrome.prototype = { * Add a listener for StyleEditorChrome events. * * The listener implements IStyleEditorChromeListener := { + * onContentAttach: Called when a content window has been attached. + * All editors are instantiated, though they might + * not be loaded yet. + * Arguments: (StyleEditorChrome aChrome) + * @see contentWindow + * @see StyleEditor.isLoaded + * @see StyleEditor.addActionListener + * * onContentDetach: Called when the content window has been detached. * Arguments: (StyleEditorChrome aChrome) * @see contentWindow @@ -280,7 +287,7 @@ StyleEditorChrome.prototype = { // (re)enable UI let matches = this._root.querySelectorAll("toolbarbutton,input,select"); - for (let i = 0; i < matches.length; i++) { + for (let i = 0; i < matches.length; ++i) { matches[i].removeAttribute("disabled"); } }, @@ -298,7 +305,7 @@ StyleEditorChrome.prototype = { this._document.title = _("chromeWindowTitle", document.title || document.location.href); - for (let i = 0; i < document.styleSheets.length; i++) { + for (let i = 0; i < document.styleSheets.length; ++i) { let styleSheet = document.styleSheets[i]; let editor = new StyleEditor(document, styleSheet); @@ -306,6 +313,8 @@ StyleEditorChrome.prototype = { this._editors.push(editor); } + this._triggerChromeListeners("ContentAttach"); + // Queue editors loading so that ContentAttach is consistently triggered // right after all editor instances are available (this.editors) but are // NOT loaded/ready yet. This also helps responsivity during loading when @@ -344,36 +353,43 @@ StyleEditorChrome.prototype = { let select = function DEC_select(aEditor) { let sheet = this._styleSheetToSelect.sheet; - let line = this._styleSheetToSelect.line || 1; - let col = this._styleSheetToSelect.col || 1; + let line = this._styleSheetToSelect.line; + let col = this._styleSheetToSelect.col; + let summary = sheet ? this.getSummaryElementForEditor(aEditor) + : this._view.getSummaryElementByOrdinal(0); - if (!aEditor.sourceEditor) { - let onAttach = function SEC_selectSheet_onAttach() { - aEditor.removeActionListener(this); - this.selectedStyleSheetIndex = aEditor.styleSheetIndex; + if (line) { + col = col || 1; + + if (!aEditor.sourceEditor) { + // If a line or column was specified we move the caret appropriately. + let self = this; + aEditor.addActionListener({ + onAttach: function SEC_selectSheet_onAttach() + { + aEditor.removeActionListener(this); + self.selectedStyleSheetIndex = aEditor.styleSheetIndex; + aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); + + let newSheet = self._styleSheetToSelect.sheet; + let newLine = self._styleSheetToSelect.line; + let newCol = self._styleSheetToSelect.col; + self._styleSheetToSelect = null; + if (newSheet != sheet) { + self._window.setTimeout(self.selectStyleSheet.bind(self, newSheet, newLine, newCol), 0); + } + } + }); + } else { + // If a line or column was specified we move the caret appropriately. aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); - - let newSheet = this._styleSheetToSelect.sheet; - let newLine = this._styleSheetToSelect.line; - let newCol = this._styleSheetToSelect.col; this._styleSheetToSelect = null; - if (newSheet != sheet) { - this.selectStyleSheet.bind(this, newSheet, newLine, newCol); - } - }.bind(this); - - aEditor.addActionListener({ - onAttach: onAttach - }); + } } else { - // If a line or column was specified we move the caret appropriately. - aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); this._styleSheetToSelect = null; } - let summary = sheet ? this.getSummaryElementForEditor(aEditor) - : this._view.getSummaryElementByOrdinal(0); - this._view.activeSummary = summary; + this._view.activeSummary = summary; this.selectedStyleSheetIndex = aEditor.styleSheetIndex; }.bind(this); @@ -388,7 +404,6 @@ StyleEditorChrome.prototype = { if ((sheet && aEditor.styleSheet == sheet) || (aEditor.styleSheetIndex == 0 && sheet == null)) { aChrome.removeChromeListener(this); - aEditor.addActionListener(self); select(aEditor); } } @@ -415,7 +430,7 @@ StyleEditorChrome.prototype = { _disableChrome: function SEC__disableChrome() { let matches = this._root.querySelectorAll("button,toolbarbutton,textbox"); - for (let i = 0; i < matches.length; i++) { + for (let i = 0; i < matches.length; ++i) { matches[i].setAttribute("disabled", "disabled"); } diff --git a/browser/devtools/styleeditor/StyleEditorPanel.jsm b/browser/devtools/styleeditor/StyleEditorPanel.jsm index f84b856d1412..7122a495fe85 100644 --- a/browser/devtools/styleeditor/StyleEditorPanel.jsm +++ b/browser/devtools/styleeditor/StyleEditorPanel.jsm @@ -39,14 +39,10 @@ StyleEditorPanel.prototype = { */ open: function StyleEditor_open() { let contentWin = this._toolbox.target.window; - let deferred = Promise.defer(); + this.setPage(contentWin); + this.isReady = true; - this.setPage(contentWin).then(function() { - this.isReady = true; - deferred.resolve(this); - }.bind(this)); - - return deferred.promise; + return Promise.resolve(this); }, /** @@ -70,16 +66,12 @@ StyleEditorPanel.prototype = { setPage: function StyleEditor_setPage(contentWindow) { if (this._panelWin.styleEditorChrome) { this._panelWin.styleEditorChrome.contentWindow = contentWindow; - this.selectStyleSheet(null, null, null); } else { let chromeRoot = this._panelDoc.getElementById("style-editor-chrome"); let chrome = new StyleEditorChrome(chromeRoot, contentWindow); - let promise = chrome.open(); - this._panelWin.styleEditorChrome = chrome; - this.selectStyleSheet(null, null, null); - return promise; } + this.selectStyleSheet(null, null, null); }, /** diff --git a/browser/devtools/styleeditor/StyleEditorUtil.jsm b/browser/devtools/styleeditor/StyleEditorUtil.jsm index f083a47c8f3d..7ef4a10c6fec 100644 --- a/browser/devtools/styleeditor/StyleEditorUtil.jsm +++ b/browser/devtools/styleeditor/StyleEditorUtil.jsm @@ -152,7 +152,7 @@ this.wire = function wire(aRoot, aSelectorOrElement, aDescriptor) aDescriptor = {events: {click: aDescriptor}}; } - for (let i = 0; i < matches.length; i++) { + for (let i = 0; i < matches.length; ++i) { let element = matches[i]; forEach(aDescriptor.events, function (aName, aHandler) { element.addEventListener(aName, aHandler, false); diff --git a/browser/devtools/styleeditor/test/Makefile.in b/browser/devtools/styleeditor/test/Makefile.in index addbb245203f..f458d19c2a6b 100644 --- a/browser/devtools/styleeditor/test/Makefile.in +++ b/browser/devtools/styleeditor/test/Makefile.in @@ -18,7 +18,7 @@ _BROWSER_TEST_FILES = \ browser_styleeditor_cmd_edit.html \ browser_styleeditor_import.js \ browser_styleeditor_init.js \ - browser_styleeditor_loading.js \ + $(filter disabled-temporarily--bug-817294, browser_styleeditor_loading.js) \ browser_styleeditor_new.js \ browser_styleeditor_passedinsheet.js \ browser_styleeditor_pretty.js \ @@ -32,7 +32,6 @@ _BROWSER_TEST_FILES = \ four.html \ head.js \ helpers.js \ - longload.html \ media.html \ media-small.css \ minified.html \ diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_import.js b/browser/devtools/styleeditor/test/browser_styleeditor_import.js index cd6db75ace81..f8d15d8a463a 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_import.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_import.js @@ -19,9 +19,12 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ + onContentAttach: run, onEditorAdded: testEditorAdded }); - run(aChrome); + if (aChrome.isContentAttached) { + run(aChrome); + } }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_init.js b/browser/devtools/styleeditor/test/browser_styleeditor_init.js index c106e3db9bc1..7835c7c15ba2 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_init.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_init.js @@ -9,17 +9,23 @@ function test() { waitForExplicitFinish(); - launchStyleEditorChrome(function(aChrome) { + addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ + onContentAttach: run, onEditorAdded: testEditorAdded }); - run(aChrome); + if (aChrome.isContentAttached) { + run(aChrome); + } }); + content.location = TESTCASE_URI; } +let gContentAttachHandled = false; function run(aChrome) { + gContentAttachHandled = true; is(aChrome.contentWindow.document.readyState, "complete", "content document is complete"); @@ -36,6 +42,11 @@ function run(aChrome) let gEditorAddedCount = 0; function testEditorAdded(aChrome, aEditor) { + if (!gEditorAddedCount) { + is(gContentAttachHandled, true, + "ContentAttach event triggered before EditorAdded"); + } + if (aEditor.styleSheetIndex == 0) { gEditorAddedCount++; testFirstStyleSheetEditor(aChrome, aEditor); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js index c422785fc84d..07331c6e092c 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js @@ -2,7 +2,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -const TESTCASE_URI = TEST_BASE + "longload.html"; +const TESTCASE_URI = TEST_BASE + "simple.html"; function test() @@ -13,25 +13,52 @@ function test() // launch Style Editor right when the tab is created (before load) // this checks that the Style Editor still launches correctly when it is opened - // *while* the page is still loading. The Style Editor should not signal that - // it is loaded until the accompanying content page is loaded. + // *while* the page is still loading launchStyleEditorChrome(function (aChrome) { content.location = TESTCASE_URI; - is(aChrome.contentWindow.document.readyState, "complete", - "content document is complete"); + executeSoon(function() { + isnot(gBrowser.selectedBrowser.contentWindow.document.readyState, "complete", + "content document is still loading"); let root = gChromeWindow.document.querySelector(".splitview-root"); - ok(!root.classList.contains("loading"), - "style editor root element does not have 'loading' class name anymore"); + ok(root.classList.contains("loading"), + "style editor root element has 'loading' class name"); let button = gChromeWindow.document.querySelector(".style-editor-newButton"); - ok(!button.hasAttribute("disabled"), - "new style sheet button is enabled"); + ok(button.hasAttribute("disabled"), + "new style sheet button is disabled"); button = gChromeWindow.document.querySelector(".style-editor-importButton"); - ok(!button.hasAttribute("disabled"), - "import button is enabled"); + ok(button.hasAttribute("disabled"), + "import button is disabled"); - finish(); + if (!aChrome.isContentAttached) { + aChrome.addChromeListener({ + onContentAttach: run + }); + } else { + run(aChrome); + } + }); }); } + +function run(aChrome) +{ + is(aChrome.contentWindow.document.readyState, "complete", + "content document is complete"); + + let root = gChromeWindow.document.querySelector(".splitview-root"); + ok(!root.classList.contains("loading"), + "style editor root element does not have 'loading' class name anymore"); + + let button = gChromeWindow.document.querySelector(".style-editor-newButton"); + ok(!button.hasAttribute("disabled"), + "new style sheet button is enabled"); + + button = gChromeWindow.document.querySelector(".style-editor-importButton"); + ok(!button.hasAttribute("disabled"), + "import button is enabled"); + + finish(); +} diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_new.js b/browser/devtools/styleeditor/test/browser_styleeditor_new.js index 3d05336123c1..f8de9da49508 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_new.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_new.js @@ -13,9 +13,12 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ + onContentAttach: run, onEditorAdded: testEditorAdded }); - run(aChrome); + if (aChrome.isContentAttached) { + run(aChrome); + } }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js index 13210fad6bcb..58ac1796b727 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js @@ -19,7 +19,13 @@ function test() { aWindow.gBrowser.selectedBrowser.removeEventListener("load", onLoad, true); cache.evictEntries(Ci.nsICache.STORE_ANYWHERE); launchStyleEditorChromeFromWindow(aWindow, function(aChrome) { - onEditorAdded(aChrome, aChrome.editors[0]); + if (aChrome.isContentAttached) { + onEditorAdded(aChrome, aChrome.editors[0]); + } else { + aChrome.addChromeListener({ + onEditorAdded: onEditorAdded + }); + } }); }, true); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js index 151df51e64db..e734f7d0237a 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js @@ -10,7 +10,12 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - run(aChrome); + aChrome.addChromeListener({ + onContentAttach: run + }); + if (aChrome.isContentAttached) { + run(aChrome); + } }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js index 37f2e7cf55b3..4c2766363b77 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js @@ -12,8 +12,15 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - run(aChrome); + if (aChrome.isContentAttached) { + run(aChrome); + } else { + aChrome.addChromeListener({ + onContentAttach: run + }); + } }); + content.location = TESTCASE_URI; } @@ -24,13 +31,13 @@ function run(aChrome) aChrome.editors[0].addActionListener({ onAttach: function onEditorAttached(aEditor) { + let originalSourceEditor = aEditor.sourceEditor; + aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved + + // queue a resize to inverse aspect ratio + // this will trigger a detach and reattach (to workaround bug 254144) executeSoon(function () { waitForFocus(function () { - // queue a resize to inverse aspect ratio - // this will trigger a detach and reattach (to workaround bug 254144) - let originalSourceEditor = aEditor.sourceEditor; - aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved - gOriginalWidth = gChromeWindow.outerWidth; gOriginalHeight = gChromeWindow.outerHeight; gChromeWindow.resizeTo(120, 480); diff --git a/browser/devtools/styleeditor/test/longload.html b/browser/devtools/styleeditor/test/longload.html deleted file mode 100644 index f4c147589754..000000000000 --- a/browser/devtools/styleeditor/test/longload.html +++ /dev/null @@ -1,28 +0,0 @@ - - - - Long load - - - - - Time passes: - - - From 99b56ebf7b3f3fcdfbb46c8b2b7596082f859a51 Mon Sep 17 00:00:00 2001 From: Anton Kovalyov Date: Wed, 6 Feb 2013 12:10:30 -0800 Subject: [PATCH 10/15] Bug 828049 - Remote profiling, r=past --- .../devtools/framework/ToolDefinitions.jsm | 2 +- .../devtools/profiler/ProfilerController.jsm | 35 +++++++++--- browser/devtools/profiler/ProfilerPanel.jsm | 2 +- browser/devtools/profiler/test/Makefile.in | 1 + .../profiler/test/browser_profiler_remote.js | 55 +++++++++++++++++++ browser/devtools/profiler/test/head.js | 13 ++++- 6 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 browser/devtools/profiler/test/browser_profiler_remote.js diff --git a/browser/devtools/framework/ToolDefinitions.jsm b/browser/devtools/framework/ToolDefinitions.jsm index 582142243fbc..7505bf54c470 100644 --- a/browser/devtools/framework/ToolDefinitions.jsm +++ b/browser/devtools/framework/ToolDefinitions.jsm @@ -153,7 +153,7 @@ let profilerDefinition = { tooltip: l10n("profiler.tooltip", profilerStrings), isTargetSupported: function (target) { - return !target.isRemote; + return true; }, build: function (frame, target) { diff --git a/browser/devtools/profiler/ProfilerController.jsm b/browser/devtools/profiler/ProfilerController.jsm index 183c7a9fb8b9..9ff3cff0f610 100644 --- a/browser/devtools/profiler/ProfilerController.jsm +++ b/browser/devtools/profiler/ProfilerController.jsm @@ -22,14 +22,21 @@ XPCOMUtils.defineLazyGetter(this, "DebuggerServer", function () { * Object acting as a mediator between the ProfilerController and * DebuggerServer. */ -function ProfilerConnection() { +function ProfilerConnection(client) { if (!DebuggerServer.initialized) { DebuggerServer.init(); DebuggerServer.addBrowserActors(); } - let transport = DebuggerServer.connectPipe(); - this.client = new DebuggerClient(transport); + this.isRemote = true; + + if (!client) { + let transport = DebuggerServer.connectPipe(); + client = new DebuggerClient(transport); + this.isRemote = false; + } + + this.client = client; } ProfilerConnection.prototype = { @@ -44,12 +51,20 @@ ProfilerConnection.prototype = { connect: function PCn_connect(aCallback) { let client = this.client; - client.connect(function (aType, aTraits) { + let listTabs = function () { client.listTabs(function (aResponse) { this.actor = aResponse.profilerActor; aCallback(); }.bind(this)); - }.bind(this)); + }.bind(this); + + if (this.isRemote) { + return void listTabs(); + } + + client.connect(function (aType, aTraits) { + listTabs(); + }); }, /** @@ -123,8 +138,14 @@ ProfilerConnection.prototype = { /** * Object defining the profiler controller components. */ -function ProfilerController() { - this.profiler = new ProfilerConnection(); +function ProfilerController(target) { + let client; + + if (target.isRemote) { + client = target.client; + } + + this.profiler = new ProfilerConnection(client); this._connected = false; } diff --git a/browser/devtools/profiler/ProfilerPanel.jsm b/browser/devtools/profiler/ProfilerPanel.jsm index 0caab1dad772..4026b1e3a4fe 100644 --- a/browser/devtools/profiler/ProfilerPanel.jsm +++ b/browser/devtools/profiler/ProfilerPanel.jsm @@ -179,7 +179,7 @@ function ProfilerPanel(frame, toolbox) { this.window = frame.window; this.document = frame.document; this.target = toolbox.target; - this.controller = new ProfilerController(); + this.controller = new ProfilerController(this.target); this.profiles = new Map(); this._uid = 0; diff --git a/browser/devtools/profiler/test/Makefile.in b/browser/devtools/profiler/test/Makefile.in index 80591222c688..e9be3166db41 100644 --- a/browser/devtools/profiler/test/Makefile.in +++ b/browser/devtools/profiler/test/Makefile.in @@ -14,6 +14,7 @@ MOCHITEST_BROWSER_FILES = \ browser_profiler_run.js \ browser_profiler_controller.js \ browser_profiler_profiles.js \ + browser_profiler_remote.js \ head.js \ include $(topsrcdir)/config/rules.mk diff --git a/browser/devtools/profiler/test/browser_profiler_remote.js b/browser/devtools/profiler/test/browser_profiler_remote.js new file mode 100644 index 000000000000..450f9d2902e5 --- /dev/null +++ b/browser/devtools/profiler/test/browser_profiler_remote.js @@ -0,0 +1,55 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +const URL = "data:text/html;charset=utf8,

JavaScript Profiler test

"; + +let temp = {}; + +Cu.import("resource://gre/modules/devtools/dbg-server.jsm", temp); +let DebuggerServer = temp.DebuggerServer; + +Cu.import("resource://gre/modules/devtools/dbg-client.jsm", temp); +let DebuggerClient = temp.DebuggerClient; +let debuggerSocketConnect = temp.debuggerSocketConnect; + +Cu.import("resource:///modules/devtools/ProfilerController.jsm", temp); +let ProfilerController = temp.ProfilerController; + +function test() { + waitForExplicitFinish(); + Services.prefs.setBoolPref(REMOTE_ENABLED, true); + + loadTab(URL, function onTabLoad(tab, browser) { + DebuggerServer.init(function () true); + DebuggerServer.addBrowserActors(); + is(DebuggerServer._socketConnections, 0); + + DebuggerServer.openListener(2929); + is(DebuggerServer._socketConnections, 1); + + let transport = debuggerSocketConnect("127.0.0.1", 2929); + let client = new DebuggerClient(transport); + client.connect(function onClientConnect() { + let target = { isRemote: true, client: client }; + let controller = new ProfilerController(target); + + controller.connect(function onControllerConnect() { + // If this callback is called, this means listTabs call worked. + // Which means that the transport worked. Time to finish up this + // test. + + function onShutdown() { + window.removeEventListener("Debugger:Shutdown", onShutdown, true); + transport = client = null; + finish(); + } + + window.addEventListener("Debugger:Shutdown", onShutdown, true); + + client.close(function () { + gBrowser.removeTab(tab); + }); + }); + }); + }); +} \ No newline at end of file diff --git a/browser/devtools/profiler/test/head.js b/browser/devtools/profiler/test/head.js index 53846e54ac6e..f2c317c61e2b 100644 --- a/browser/devtools/profiler/test/head.js +++ b/browser/devtools/profiler/test/head.js @@ -3,6 +3,7 @@ let temp = {}; const PROFILER_ENABLED = "devtools.profiler.enabled"; +const REMOTE_ENABLED = "devtools.debugger.remote-enabled"; Cu.import("resource:///modules/devtools/Target.jsm", temp); let TargetFactory = temp.TargetFactory; @@ -10,6 +11,15 @@ let TargetFactory = temp.TargetFactory; Cu.import("resource:///modules/devtools/gDevTools.jsm", temp); let gDevTools = temp.gDevTools; +Cu.import("resource://gre/modules/devtools/dbg-server.jsm", temp); +let DebuggerServer = temp.DebuggerServer; + +registerCleanupFunction(function () { + Services.prefs.clearUserPref(PROFILER_ENABLED); + Services.prefs.clearUserPref(REMOTE_ENABLED); + DebuggerServer.destroy(); +}); + function loadTab(url, callback) { let tab = gBrowser.addTab(); gBrowser.selectedTab = tab; @@ -63,6 +73,5 @@ function tearDown(tab, callback=function(){}) { } finish(); - Services.prefs.setBoolPref(PROFILER_ENABLED, false); }); -} +} \ No newline at end of file From 8cec10d191c32184b8dd47e4d1a91e51bb7bc616 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 6 Feb 2013 22:17:05 +0000 Subject: [PATCH 11/15] Bug 817294 - Intermittent browser_styleeditor_loading.js | content document is still loading - Didn't expect complete, but got it | style editor root element has 'loading' class name | new style sheet button is disabled | import button is disabled --- browser/devtools/styleeditor/StyleEditor.jsm | 5 +- .../styleeditor/StyleEditorChrome.jsm | 133 ++++++++---------- .../devtools/styleeditor/StyleEditorPanel.jsm | 16 ++- .../devtools/styleeditor/StyleEditorUtil.jsm | 2 +- browser/devtools/styleeditor/test/Makefile.in | 3 +- .../test/browser_styleeditor_import.js | 5 +- .../test/browser_styleeditor_init.js | 15 +- .../test/browser_styleeditor_loading.js | 51 ++----- .../test/browser_styleeditor_new.js | 5 +- ...browser_styleeditor_private_perwindowpb.js | 8 +- .../test/browser_styleeditor_sv_keynav.js | 7 +- .../test/browser_styleeditor_sv_resize.js | 19 +-- .../devtools/styleeditor/test/longload.html | 28 ++++ 13 files changed, 129 insertions(+), 168 deletions(-) create mode 100644 browser/devtools/styleeditor/test/longload.html diff --git a/browser/devtools/styleeditor/StyleEditor.jsm b/browser/devtools/styleeditor/StyleEditor.jsm index 8052b14d72ac..db8aaabe56c1 100644 --- a/browser/devtools/styleeditor/StyleEditor.jsm +++ b/browser/devtools/styleeditor/StyleEditor.jsm @@ -130,7 +130,7 @@ StyleEditor.prototype = { { let document = this.contentDocument; if (this._styleSheetIndex == -1) { - for (let i = 0; i < document.styleSheets.length; ++i) { + for (let i = 0; i < document.styleSheets.length; i++) { if (document.styleSheets[i] == this.styleSheet) { this._styleSheetIndex = i; break; @@ -1004,8 +1004,9 @@ StyleEditor.prototype = { // copy the list of listeners to allow adding/removing listeners in handlers let listeners = this._actionListeners.concat(); + // trigger all listeners that have this action handler - for (let i = 0; i < listeners.length; ++i) { + for (let i = 0; i < listeners.length; i++) { let listener = listeners[i]; let actionHandler = listener["on" + aName]; if (actionHandler) { diff --git a/browser/devtools/styleeditor/StyleEditorChrome.jsm b/browser/devtools/styleeditor/StyleEditorChrome.jsm index a8da29eb837a..860144b5f273 100644 --- a/browser/devtools/styleeditor/StyleEditorChrome.jsm +++ b/browser/devtools/styleeditor/StyleEditorChrome.jsm @@ -16,6 +16,7 @@ Cu.import("resource://gre/modules/PluralForm.jsm"); Cu.import("resource:///modules/devtools/StyleEditor.jsm"); Cu.import("resource:///modules/devtools/StyleEditorUtil.jsm"); Cu.import("resource:///modules/devtools/SplitView.jsm"); +Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js"); const STYLE_EDITOR_TEMPLATE = "stylesheet"; @@ -43,34 +44,43 @@ this.StyleEditorChrome = function StyleEditorChrome(aRoot, aContentWindow) this._editors = []; this._listeners = []; // @see addChromeListener + // Store the content window so that we can call the real contentWindow setter + // in the open method. + this._contentWindowTemp = aContentWindow; + this._contentWindow = null; - this._isContentAttached = false; - - let initializeUI = function (aEvent) { - if (aEvent) { - this._window.removeEventListener("load", initializeUI, false); - } - - let viewRoot = this._root.parentNode.querySelector(".splitview-root"); - this._view = new SplitView(viewRoot); - - this._setupChrome(); - - // attach to the content window - this.contentWindow = aContentWindow; - this._contentWindowID = null; - }.bind(this); - - if (this._document.readyState == "complete") { - initializeUI(); - } else { - this._window.addEventListener("load", initializeUI, false); - } } StyleEditorChrome.prototype = { _styleSheetToSelect: null, + open: function() { + let deferred = Promise.defer(); + let initializeUI = function (aEvent) { + if (aEvent) { + this._window.removeEventListener("load", initializeUI, false); + } + let viewRoot = this._root.parentNode.querySelector(".splitview-root"); + this._view = new SplitView(viewRoot); + this._setupChrome(); + + // We need to juggle arount the contentWindow items because we need to + // trigger the setter at the appropriate time. + this.contentWindow = this._contentWindowTemp; // calls setter + this._contentWindowTemp = null; + + deferred.resolve(); + }.bind(this); + + if (this._document.readyState == "complete") { + initializeUI(); + } else { + this._window.addEventListener("load", initializeUI, false); + } + + return deferred.promise; + }, + /** * Retrieve the content window attached to this chrome. * @@ -150,15 +160,6 @@ StyleEditorChrome.prototype = { return this._contentWindow ? this._contentWindow.document : null; }, - /** - * Retrieve whether the content has been attached and StyleEditor instances - * exist for all of its stylesheets. - * - * @return boolean - * @see addChromeListener - */ - get isContentAttached() this._isContentAttached, - /** * Retrieve an array with the StyleEditor instance for each live style sheet, * ordered by style sheet index. @@ -180,14 +181,6 @@ StyleEditorChrome.prototype = { * Add a listener for StyleEditorChrome events. * * The listener implements IStyleEditorChromeListener := { - * onContentAttach: Called when a content window has been attached. - * All editors are instantiated, though they might - * not be loaded yet. - * Arguments: (StyleEditorChrome aChrome) - * @see contentWindow - * @see StyleEditor.isLoaded - * @see StyleEditor.addActionListener - * * onContentDetach: Called when the content window has been detached. * Arguments: (StyleEditorChrome aChrome) * @see contentWindow @@ -287,7 +280,7 @@ StyleEditorChrome.prototype = { // (re)enable UI let matches = this._root.querySelectorAll("toolbarbutton,input,select"); - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { matches[i].removeAttribute("disabled"); } }, @@ -305,7 +298,7 @@ StyleEditorChrome.prototype = { this._document.title = _("chromeWindowTitle", document.title || document.location.href); - for (let i = 0; i < document.styleSheets.length; ++i) { + for (let i = 0; i < document.styleSheets.length; i++) { let styleSheet = document.styleSheets[i]; let editor = new StyleEditor(document, styleSheet); @@ -313,8 +306,6 @@ StyleEditorChrome.prototype = { this._editors.push(editor); } - this._triggerChromeListeners("ContentAttach"); - // Queue editors loading so that ContentAttach is consistently triggered // right after all editor instances are available (this.editors) but are // NOT loaded/ready yet. This also helps responsivity during loading when @@ -353,43 +344,36 @@ StyleEditorChrome.prototype = { let select = function DEC_select(aEditor) { let sheet = this._styleSheetToSelect.sheet; - let line = this._styleSheetToSelect.line; - let col = this._styleSheetToSelect.col; - let summary = sheet ? this.getSummaryElementForEditor(aEditor) - : this._view.getSummaryElementByOrdinal(0); + let line = this._styleSheetToSelect.line || 1; + let col = this._styleSheetToSelect.col || 1; - if (line) { - col = col || 1; - - if (!aEditor.sourceEditor) { - // If a line or column was specified we move the caret appropriately. - let self = this; - aEditor.addActionListener({ - onAttach: function SEC_selectSheet_onAttach() - { - aEditor.removeActionListener(this); - self.selectedStyleSheetIndex = aEditor.styleSheetIndex; - aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); - - let newSheet = self._styleSheetToSelect.sheet; - let newLine = self._styleSheetToSelect.line; - let newCol = self._styleSheetToSelect.col; - self._styleSheetToSelect = null; - if (newSheet != sheet) { - self._window.setTimeout(self.selectStyleSheet.bind(self, newSheet, newLine, newCol), 0); - } - } - }); - } else { - // If a line or column was specified we move the caret appropriately. + if (!aEditor.sourceEditor) { + let onAttach = function SEC_selectSheet_onAttach() { + aEditor.removeActionListener(this); + this.selectedStyleSheetIndex = aEditor.styleSheetIndex; aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); + + let newSheet = this._styleSheetToSelect.sheet; + let newLine = this._styleSheetToSelect.line; + let newCol = this._styleSheetToSelect.col; this._styleSheetToSelect = null; - } + if (newSheet != sheet) { + this.selectStyleSheet.bind(this, newSheet, newLine, newCol); + } + }.bind(this); + + aEditor.addActionListener({ + onAttach: onAttach + }); } else { + // If a line or column was specified we move the caret appropriately. + aEditor.sourceEditor.setCaretPosition(line - 1, col - 1); this._styleSheetToSelect = null; } - this._view.activeSummary = summary; + let summary = sheet ? this.getSummaryElementForEditor(aEditor) + : this._view.getSummaryElementByOrdinal(0); + this._view.activeSummary = summary; this.selectedStyleSheetIndex = aEditor.styleSheetIndex; }.bind(this); @@ -404,6 +388,7 @@ StyleEditorChrome.prototype = { if ((sheet && aEditor.styleSheet == sheet) || (aEditor.styleSheetIndex == 0 && sheet == null)) { aChrome.removeChromeListener(this); + aEditor.addActionListener(self); select(aEditor); } } @@ -430,7 +415,7 @@ StyleEditorChrome.prototype = { _disableChrome: function SEC__disableChrome() { let matches = this._root.querySelectorAll("button,toolbarbutton,textbox"); - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { matches[i].setAttribute("disabled", "disabled"); } diff --git a/browser/devtools/styleeditor/StyleEditorPanel.jsm b/browser/devtools/styleeditor/StyleEditorPanel.jsm index 7122a495fe85..f84b856d1412 100644 --- a/browser/devtools/styleeditor/StyleEditorPanel.jsm +++ b/browser/devtools/styleeditor/StyleEditorPanel.jsm @@ -39,10 +39,14 @@ StyleEditorPanel.prototype = { */ open: function StyleEditor_open() { let contentWin = this._toolbox.target.window; - this.setPage(contentWin); - this.isReady = true; + let deferred = Promise.defer(); - return Promise.resolve(this); + this.setPage(contentWin).then(function() { + this.isReady = true; + deferred.resolve(this); + }.bind(this)); + + return deferred.promise; }, /** @@ -66,12 +70,16 @@ StyleEditorPanel.prototype = { setPage: function StyleEditor_setPage(contentWindow) { if (this._panelWin.styleEditorChrome) { this._panelWin.styleEditorChrome.contentWindow = contentWindow; + this.selectStyleSheet(null, null, null); } else { let chromeRoot = this._panelDoc.getElementById("style-editor-chrome"); let chrome = new StyleEditorChrome(chromeRoot, contentWindow); + let promise = chrome.open(); + this._panelWin.styleEditorChrome = chrome; + this.selectStyleSheet(null, null, null); + return promise; } - this.selectStyleSheet(null, null, null); }, /** diff --git a/browser/devtools/styleeditor/StyleEditorUtil.jsm b/browser/devtools/styleeditor/StyleEditorUtil.jsm index 7ef4a10c6fec..f083a47c8f3d 100644 --- a/browser/devtools/styleeditor/StyleEditorUtil.jsm +++ b/browser/devtools/styleeditor/StyleEditorUtil.jsm @@ -152,7 +152,7 @@ this.wire = function wire(aRoot, aSelectorOrElement, aDescriptor) aDescriptor = {events: {click: aDescriptor}}; } - for (let i = 0; i < matches.length; ++i) { + for (let i = 0; i < matches.length; i++) { let element = matches[i]; forEach(aDescriptor.events, function (aName, aHandler) { element.addEventListener(aName, aHandler, false); diff --git a/browser/devtools/styleeditor/test/Makefile.in b/browser/devtools/styleeditor/test/Makefile.in index f458d19c2a6b..addbb245203f 100644 --- a/browser/devtools/styleeditor/test/Makefile.in +++ b/browser/devtools/styleeditor/test/Makefile.in @@ -18,7 +18,7 @@ _BROWSER_TEST_FILES = \ browser_styleeditor_cmd_edit.html \ browser_styleeditor_import.js \ browser_styleeditor_init.js \ - $(filter disabled-temporarily--bug-817294, browser_styleeditor_loading.js) \ + browser_styleeditor_loading.js \ browser_styleeditor_new.js \ browser_styleeditor_passedinsheet.js \ browser_styleeditor_pretty.js \ @@ -32,6 +32,7 @@ _BROWSER_TEST_FILES = \ four.html \ head.js \ helpers.js \ + longload.html \ media.html \ media-small.css \ minified.html \ diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_import.js b/browser/devtools/styleeditor/test/browser_styleeditor_import.js index f8d15d8a463a..cd6db75ace81 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_import.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_import.js @@ -19,12 +19,9 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_init.js b/browser/devtools/styleeditor/test/browser_styleeditor_init.js index 7835c7c15ba2..c106e3db9bc1 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_init.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_init.js @@ -9,23 +9,17 @@ function test() { waitForExplicitFinish(); - addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { + launchStyleEditorChrome(function(aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); - content.location = TESTCASE_URI; } -let gContentAttachHandled = false; function run(aChrome) { - gContentAttachHandled = true; is(aChrome.contentWindow.document.readyState, "complete", "content document is complete"); @@ -42,11 +36,6 @@ function run(aChrome) let gEditorAddedCount = 0; function testEditorAdded(aChrome, aEditor) { - if (!gEditorAddedCount) { - is(gContentAttachHandled, true, - "ContentAttach event triggered before EditorAdded"); - } - if (aEditor.styleSheetIndex == 0) { gEditorAddedCount++; testFirstStyleSheetEditor(aChrome, aEditor); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js index 07331c6e092c..c422785fc84d 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_loading.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_loading.js @@ -2,7 +2,7 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -const TESTCASE_URI = TEST_BASE + "simple.html"; +const TESTCASE_URI = TEST_BASE + "longload.html"; function test() @@ -13,52 +13,25 @@ function test() // launch Style Editor right when the tab is created (before load) // this checks that the Style Editor still launches correctly when it is opened - // *while* the page is still loading + // *while* the page is still loading. The Style Editor should not signal that + // it is loaded until the accompanying content page is loaded. launchStyleEditorChrome(function (aChrome) { content.location = TESTCASE_URI; - executeSoon(function() { - isnot(gBrowser.selectedBrowser.contentWindow.document.readyState, "complete", - "content document is still loading"); + is(aChrome.contentWindow.document.readyState, "complete", + "content document is complete"); let root = gChromeWindow.document.querySelector(".splitview-root"); - ok(root.classList.contains("loading"), - "style editor root element has 'loading' class name"); + ok(!root.classList.contains("loading"), + "style editor root element does not have 'loading' class name anymore"); let button = gChromeWindow.document.querySelector(".style-editor-newButton"); - ok(button.hasAttribute("disabled"), - "new style sheet button is disabled"); + ok(!button.hasAttribute("disabled"), + "new style sheet button is enabled"); button = gChromeWindow.document.querySelector(".style-editor-importButton"); - ok(button.hasAttribute("disabled"), - "import button is disabled"); + ok(!button.hasAttribute("disabled"), + "import button is enabled"); - if (!aChrome.isContentAttached) { - aChrome.addChromeListener({ - onContentAttach: run - }); - } else { - run(aChrome); - } - }); + finish(); }); } - -function run(aChrome) -{ - is(aChrome.contentWindow.document.readyState, "complete", - "content document is complete"); - - let root = gChromeWindow.document.querySelector(".splitview-root"); - ok(!root.classList.contains("loading"), - "style editor root element does not have 'loading' class name anymore"); - - let button = gChromeWindow.document.querySelector(".style-editor-newButton"); - ok(!button.hasAttribute("disabled"), - "new style sheet button is enabled"); - - button = gChromeWindow.document.querySelector(".style-editor-importButton"); - ok(!button.hasAttribute("disabled"), - "import button is enabled"); - - finish(); -} diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_new.js b/browser/devtools/styleeditor/test/browser_styleeditor_new.js index f8de9da49508..3d05336123c1 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_new.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_new.js @@ -13,12 +13,9 @@ function test() addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { aChrome.addChromeListener({ - onContentAttach: run, onEditorAdded: testEditorAdded }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js index 58ac1796b727..13210fad6bcb 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js @@ -19,13 +19,7 @@ function test() { aWindow.gBrowser.selectedBrowser.removeEventListener("load", onLoad, true); cache.evictEntries(Ci.nsICache.STORE_ANYWHERE); launchStyleEditorChromeFromWindow(aWindow, function(aChrome) { - if (aChrome.isContentAttached) { - onEditorAdded(aChrome, aChrome.editors[0]); - } else { - aChrome.addChromeListener({ - onEditorAdded: onEditorAdded - }); - } + onEditorAdded(aChrome, aChrome.editors[0]); }); }, true); diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js index e734f7d0237a..151df51e64db 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_keynav.js @@ -10,12 +10,7 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - aChrome.addChromeListener({ - onContentAttach: run - }); - if (aChrome.isContentAttached) { - run(aChrome); - } + run(aChrome); }); content.location = TESTCASE_URI; diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js index 4c2766363b77..37f2e7cf55b3 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_sv_resize.js @@ -12,15 +12,8 @@ function test() waitForExplicitFinish(); addTabAndLaunchStyleEditorChromeWhenLoaded(function (aChrome) { - if (aChrome.isContentAttached) { - run(aChrome); - } else { - aChrome.addChromeListener({ - onContentAttach: run - }); - } + run(aChrome); }); - content.location = TESTCASE_URI; } @@ -31,13 +24,13 @@ function run(aChrome) aChrome.editors[0].addActionListener({ onAttach: function onEditorAttached(aEditor) { - let originalSourceEditor = aEditor.sourceEditor; - aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved - - // queue a resize to inverse aspect ratio - // this will trigger a detach and reattach (to workaround bug 254144) executeSoon(function () { waitForFocus(function () { + // queue a resize to inverse aspect ratio + // this will trigger a detach and reattach (to workaround bug 254144) + let originalSourceEditor = aEditor.sourceEditor; + aEditor.sourceEditor.setCaretOffset(4); // to check the caret is preserved + gOriginalWidth = gChromeWindow.outerWidth; gOriginalHeight = gChromeWindow.outerHeight; gChromeWindow.resizeTo(120, 480); diff --git a/browser/devtools/styleeditor/test/longload.html b/browser/devtools/styleeditor/test/longload.html new file mode 100644 index 000000000000..f4c147589754 --- /dev/null +++ b/browser/devtools/styleeditor/test/longload.html @@ -0,0 +1,28 @@ + + + + Long load + + + + + Time passes: + + + From c589a602ce29af97ff8e84d401cffd6455add65c Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Thu, 7 Feb 2013 16:18:08 +0200 Subject: [PATCH 12/15] Bug 837052 - Editing or deleting getters and setters appears to be allowed even if no eval method is provided, r=past --- browser/devtools/shared/VariablesView.jsm | 16 ++++++++++++++-- browser/themes/winstripe/devtools/debugger.css | 1 - 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/browser/devtools/shared/VariablesView.jsm b/browser/devtools/shared/VariablesView.jsm index f7d250ad4040..c7d4ea0b8d7d 100644 --- a/browser/devtools/shared/VariablesView.jsm +++ b/browser/devtools/shared/VariablesView.jsm @@ -2140,8 +2140,20 @@ create({ constructor: Variable, proto: Scope.prototype }, { separatorLabel.hidden = true; valueLabel.hidden = true; - this.delete = VariablesView.getterOrSetterDeleteCallback; - this.evaluationMacro = VariablesView.overrideValueEvalMacro; + // Changing getter/setter names is never allowed. + this.switch = null; + + // Getter/setter properties require special handling when it comes to + // evaluation and deletion. + if (this.ownerView.eval) { + this.delete = VariablesView.getterOrSetterDeleteCallback; + this.evaluationMacro = VariablesView.overrideValueEvalMacro; + } + // Deleting getters and setters individually is not allowed if no + // evaluation method is provided. + else { + this.delete = null; + } let getter = this.addProperty("get", { value: descriptor.get }); let setter = this.addProperty("set", { value: descriptor.set }); diff --git a/browser/themes/winstripe/devtools/debugger.css b/browser/themes/winstripe/devtools/debugger.css index f84ebd0102fc..180f29703bd5 100644 --- a/browser/themes/winstripe/devtools/debugger.css +++ b/browser/themes/winstripe/devtools/debugger.css @@ -428,7 +428,6 @@ .property > .title > .value { -moz-padding-start: 6px; -moz-padding-end: 4px; - cursor: text; } .property[editable] > .title > .value { From 391096d41f2844c6a2489bde6456a64832c7df1d Mon Sep 17 00:00:00 2001 From: Anton Kovalyov Date: Thu, 7 Feb 2013 11:09:55 -0800 Subject: [PATCH 13/15] Bug 830668 - Add a keyboard shortcut for the Profiler, r=robcee --- browser/devtools/framework/ToolDefinitions.jsm | 4 ++++ .../en-US/chrome/browser/devtools/profiler.properties | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/browser/devtools/framework/ToolDefinitions.jsm b/browser/devtools/framework/ToolDefinitions.jsm index 7505bf54c470..a6a0d842a7b1 100644 --- a/browser/devtools/framework/ToolDefinitions.jsm +++ b/browser/devtools/framework/ToolDefinitions.jsm @@ -146,6 +146,10 @@ let styleEditorDefinition = { let profilerDefinition = { id: "jsprofiler", + accesskey: l10n("profiler.accesskey", profilerStrings), + key: l10n("profiler.commandkey", profilerStrings), + ordinal: 4, + modifiers: osString == "Darwin" ? "accel,alt" : "accel,shift", killswitch: "devtools.profiler.enabled", url: "chrome://browser/content/profiler.xul", label: l10n("profiler.label", profilerStrings), diff --git a/browser/locales/en-US/chrome/browser/devtools/profiler.properties b/browser/locales/en-US/chrome/browser/devtools/profiler.properties index 62fbd607b7eb..6736ad307e57 100644 --- a/browser/locales/en-US/chrome/browser/devtools/profiler.properties +++ b/browser/locales/en-US/chrome/browser/devtools/profiler.properties @@ -15,6 +15,11 @@ # displayed inside the developer tools window and in the Developer Tools Menu. profiler.label=Profiler +# LOCALIZATION NOTE (profiler.commandkey, profiler.accesskey) +# Used for the menuitem in the tool menu +profiler.commandkey=Y +profiler.accesskey=Y + # LOCALIZATION NOTE (profiler.tooltip): # This string is displayed in the tooltip of the tab when the profiler is # displayed inside the developer tools window. @@ -41,6 +46,9 @@ profiler.runningTime=Running Time # LOCALIZATION NOTE (profiler.self): # This string is displayed as a table header in the profiler UI. +# "Self" is how much time was spent doing work directly in that function. +# As opposed to the total time which is how much time was spent in that +# function and in functions it called. profiler.self=Self # LOCALIZATION NOTE (profiler.symbolName) From 5b718d40f8dbe72e51d33d13bc5a3a293681bd3a Mon Sep 17 00:00:00 2001 From: Anton Kovalyov Date: Thu, 7 Feb 2013 13:02:38 -0800 Subject: [PATCH 14/15] Bug 837735 - Replace Cleopatra progress strings with 'Loading profile...', r=robcee --- .../profiler/cleopatra/js/devtools.js | 31 +++++++++++++++++++ .../browser/devtools/profiler.properties | 7 ++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/browser/devtools/profiler/cleopatra/js/devtools.js b/browser/devtools/profiler/cleopatra/js/devtools.js index 544aae0aeb9d..3eb4c856391d 100644 --- a/browser/devtools/profiler/cleopatra/js/devtools.js +++ b/browser/devtools/profiler/cleopatra/js/devtools.js @@ -213,4 +213,35 @@ function enterFinishedProfileUI() { } toggleJavascriptOnly(); +} + +function enterProgressUI() { + var pane = document.createElement("div"); + var label = document.createElement("a"); + var bar = document.createElement("progress"); + var string = gStrings.getStr("profiler.loading"); + + pane.className = "profileProgressPane"; + pane.appendChild(label); + pane.appendChild(bar); + + var reporter = new ProgressReporter(); + reporter.addListener(function (rep) { + var progress = rep.getProgress(); + + if (label.textContent !== string) { + label.textContent = string; + } + + if (isNaN(progress)) { + bar.removeAttribute("value"); + } else { + bar.value = progress; + } + }); + + gMainArea.appendChild(pane); + Parser.updateLogSetting(); + + return reporter; } \ No newline at end of file diff --git a/browser/locales/en-US/chrome/browser/devtools/profiler.properties b/browser/locales/en-US/chrome/browser/devtools/profiler.properties index 6736ad307e57..1d27680209fa 100644 --- a/browser/locales/en-US/chrome/browser/devtools/profiler.properties +++ b/browser/locales/en-US/chrome/browser/devtools/profiler.properties @@ -71,4 +71,9 @@ profiler.start=Start # LOCALIZATION NOTE (profiler.stop) # This string is displayed on the button that stops the profiler. -profiler.stop=Stop \ No newline at end of file +profiler.stop=Stop + +# LOCALIZATION NOTE (profiler.loading) +# This string is displayed above the progress bar when the profiler +# is busy loading and parsing the report. +profiler.loading=Loading profile… \ No newline at end of file From f0d608dcfebcf82406a2dc57435a7d4c367ec507 Mon Sep 17 00:00:00 2001 From: Anton Kovalyov Date: Thu, 7 Feb 2013 13:22:48 -0800 Subject: [PATCH 15/15] Bug 830664, Disallow multiple profiles to run in the same time, r=robcee --- browser/devtools/profiler/ProfilerPanel.jsm | 101 ++++++++++++++---- .../profiler/cleopatra/css/devtools.css | 5 + .../profiler/cleopatra/js/devtools.js | 33 ++++-- browser/devtools/profiler/test/Makefile.in | 1 + ...r_profiler_bug_830664_multiple_profiles.js | 83 ++++++++++++++ .../profiler/test/browser_profiler_run.js | 7 -- browser/devtools/profiler/test/head.js | 8 ++ .../browser/devtools/profiler.properties | 7 +- 8 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 browser/devtools/profiler/test/browser_profiler_bug_830664_multiple_profiles.js diff --git a/browser/devtools/profiler/ProfilerPanel.jsm b/browser/devtools/profiler/ProfilerPanel.jsm index 4026b1e3a4fe..0c3efd76ed35 100644 --- a/browser/devtools/profiler/ProfilerPanel.jsm +++ b/browser/devtools/profiler/ProfilerPanel.jsm @@ -27,10 +27,13 @@ XPCOMUtils.defineLazyGetter(this, "DebuggerServer", function () { * Its main function is to talk to the Cleopatra instance * inside of the iframe. * - * ProfileUI is also an event emitter. Currently, it emits - * only one event, 'ready', when Cleopatra is done loading. - * You can also check 'isReady' property to see if a - * particular instance has been loaded yet. + * ProfileUI is also an event emitter. It emits the following events: + * - ready, when Cleopatra is done loading (you can also check the isReady + * property to see if a particular instance has been loaded yet. + * - disabled, when Cleopatra gets disabled. Happens when another ProfileUI + * instance starts the profiler. + * - enabled, when Cleopatra gets enabled. Happens when another ProfileUI + * instance stops the profiler. * * @param number uid * Unique ID for this profile. @@ -63,27 +66,45 @@ function ProfileUI(uid, panel) { return; } + let label = doc.querySelector("li#profile-" + this.uid + " > h1"); switch (event.data.status) { case "loaded": + if (this.panel._runningUid !== null) { + this.iframe.contentWindow.postMessage(JSON.stringify({ + uid: this._runningUid, + isCurrent: this._runningUid === uid, + task: "onStarted" + }), "*"); + } + this.isReady = true; this.emit("ready"); break; case "start": - // Start profiling and, once started, notify the - // underlying page so that it could update the UI. + // Start profiling and, once started, notify the underlying page + // so that it could update the UI. Also, once started, we add a + // star to the profile name to indicate which profile is currently + // running. this.panel.startProfiling(function onStart() { - var data = JSON.stringify({task: "onStarted"}); - this.iframe.contentWindow.postMessage(data, "*"); + this.panel.broadcast(this.uid, {task: "onStarted"}); + label.textContent = label.textContent + " *"; }.bind(this)); break; case "stop": - // Stop profiling and, once stopped, notify the - // underlying page so that it could update the UI. + // Stop profiling and, once stopped, notify the underlying page so + // that it could update the UI and remove a star from the profile + // name. this.panel.stopProfiling(function onStop() { - var data = JSON.stringify({task: "onStopped"}); - this.iframe.contentWindow.postMessage(data, "*"); + this.panel.broadcast(this.uid, {task: "onStopped"}); + label.textContent = label.textContent.replace(/\s\*$/, ""); }.bind(this)); + break; + case "disabled": + this.emit("disabled"); + break; + case "enabled": + this.emit("enabled"); } }.bind(this)); } @@ -188,15 +209,16 @@ function ProfilerPanel(frame, toolbox) { } ProfilerPanel.prototype = { - isReady: null, - window: null, - document: null, - target: null, - controller: null, - profiles: null, + isReady: null, + window: null, + document: null, + target: null, + controller: null, + profiles: null, - _uid: null, - _activeUid: null, + _uid: null, + _activeUid: null, + _runningUid: null, get activeProfile() { return this.profiles.get(this._activeUid); @@ -207,8 +229,8 @@ ProfilerPanel.prototype = { }, /** - * Open a debug connection and, on success, switch to - * the newly created profile. + * Open a debug connection and, on success, switch to the newly created + * profile. * * @return Promise */ @@ -359,6 +381,41 @@ ProfilerPanel.prototype = { }.bind(this)); }, + /** + * Broadcast messages to all Cleopatra instances. + * + * @param number target + * UID of the recepient profile. All profiles will receive the message + * but the profile specified by 'target' will have a special property, + * isCurrent, set to true. + * @param object data + * An object with a property 'task' that will be sent over to Cleopatra. + */ + broadcast: function PP_broadcast(target, data) { + if (!this.profiles) { + return; + } + + if (data.task === "onStarted") { + this._runningUid = target; + } else { + this._runningUid = null; + } + + let uid = this._uid; + while (uid >= 0) { + if (this.profiles.has(uid)) { + let iframe = this.profiles.get(uid).iframe; + iframe.contentWindow.postMessage(JSON.stringify({ + uid: target, + isCurrent: target === uid, + task: data.task + }), "*"); + } + uid -= 1; + } + }, + /** * Cleanup. */ diff --git a/browser/devtools/profiler/cleopatra/css/devtools.css b/browser/devtools/profiler/cleopatra/css/devtools.css index 8f898ddaf619..79b81e5a3171 100644 --- a/browser/devtools/profiler/cleopatra/css/devtools.css +++ b/browser/devtools/profiler/cleopatra/css/devtools.css @@ -10,4 +10,9 @@ #stopWrapper { display: none; +} + +#profilerMessage { + color: #999; + display: none; } \ No newline at end of file diff --git a/browser/devtools/profiler/cleopatra/js/devtools.js b/browser/devtools/profiler/cleopatra/js/devtools.js index 3eb4c856391d..f84eacc4c520 100644 --- a/browser/devtools/profiler/cleopatra/js/devtools.js +++ b/browser/devtools/profiler/cleopatra/js/devtools.js @@ -13,6 +13,8 @@ var gInstanceUID; * - loaded, when page is loaded. * - start, when user wants to start profiling. * - stop, when user wants to stop profiling. + * - disabled, when the profiler was disabled + * - enabled, when the profiler was enabled */ function notifyParent(status) { if (!gInstanceUID) { @@ -45,18 +47,34 @@ function notifyParent(status) { function onParentMessage(event) { var start = document.getElementById("startWrapper"); var stop = document.getElementById("stopWrapper"); + var profilerMessage = document.getElementById("profilerMessage"); var msg = JSON.parse(event.data); switch (msg.task) { case "onStarted": - start.style.display = "none"; - start.querySelector("button").removeAttribute("disabled"); - stop.style.display = "inline"; + if (msg.isCurrent) { + start.style.display = "none"; + start.querySelector("button").removeAttribute("disabled"); + stop.style.display = "inline"; + } else { + start.querySelector("button").setAttribute("disabled", true); + var text = gStrings.getFormatStr("profiler.alreadyRunning", [msg.uid]); + profilerMessage.textContent = text; + profilerMessage.style.display = "block"; + notifyParent("disabled"); + } break; case "onStopped": - stop.style.display = "none"; - stop.querySelector("button").removeAttribute("disabled"); - start.style.display = "inline"; + if (msg.isCurrent) { + stop.style.display = "none"; + stop.querySelector("button").removeAttribute("disabled"); + start.style.display = "inline"; + } else { + start.querySelector("button").removeAttribute("disabled"); + profilerMessage.textContent = ""; + profilerMessage.style.display = "none"; + notifyParent("enabled"); + } break; case "receiveProfileData": loadProfile(JSON.stringify(msg.rawProfile)); @@ -107,7 +125,8 @@ function initUI() { controlPane.className = "controlPane"; controlPane.innerHTML = "

" + startProfiling + "

" + - "

" + stopProfiling + "

"; + "

" + stopProfiling + "

" + + "

"; controlPane.querySelector("#startWrapper > span.btn").appendChild(startButton); controlPane.querySelector("#stopWrapper > span.btn").appendChild(stopButton); diff --git a/browser/devtools/profiler/test/Makefile.in b/browser/devtools/profiler/test/Makefile.in index e9be3166db41..44a6cb8f4a77 100644 --- a/browser/devtools/profiler/test/Makefile.in +++ b/browser/devtools/profiler/test/Makefile.in @@ -15,6 +15,7 @@ MOCHITEST_BROWSER_FILES = \ browser_profiler_controller.js \ browser_profiler_profiles.js \ browser_profiler_remote.js \ + browser_profiler_bug_830664_multiple_profiles.js \ head.js \ include $(topsrcdir)/config/rules.mk diff --git a/browser/devtools/profiler/test/browser_profiler_bug_830664_multiple_profiles.js b/browser/devtools/profiler/test/browser_profiler_bug_830664_multiple_profiles.js new file mode 100644 index 000000000000..afe6246298fc --- /dev/null +++ b/browser/devtools/profiler/test/browser_profiler_bug_830664_multiple_profiles.js @@ -0,0 +1,83 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +const URL = "data:text/html;charset=utf8,

JavaScript Profiler test

"; + +let gTab, gPanel, gUid; + +function test() { + waitForExplicitFinish(); + + setUp(URL, function onSetUp(tab, browser, panel) { + gTab = tab; + gPanel = panel; + + gPanel.once("profileCreated", function (_, uid) { + gUid = uid; + let profile = gPanel.profiles.get(uid); + + if (profile.isReady) { + startProfiling(); + } else { + profile.once("ready", startProfiling); + } + }); + gPanel.createProfile(); + }); +} + +function getCleoControls(doc) { + return [ + doc.querySelector("#startWrapper button"), + doc.querySelector("#profilerMessage") + ]; +} + +function sendFromActiveProfile(msg) { + let [win, doc] = getProfileInternals(); + + win.parent.postMessage({ + uid: gPanel.activeProfile.uid, + status: msg + }, "*"); +} + +function startProfiling() { + gPanel.profiles.get(gUid).once("disabled", stopProfiling); + sendFromActiveProfile("start"); +} + +function stopProfiling() { + let [win, doc] = getProfileInternals(gUid); + let [btn, msg] = getCleoControls(doc); + + ok(msg.textContent.match("Profile 1") !== null, "Message is visible"); + ok(btn.hasAttribute("disabled"), "Button is disabled"); + + is(gPanel.document.querySelector("li#profile-1 > h1").textContent, + "Profile 1 *", "Profile 1 has a star next to it."); + is(gPanel.document.querySelector("li#profile-2 > h1").textContent, + "Profile 2", "Profile 2 doesn't have a star next to it."); + + gPanel.profiles.get(gUid).once("enabled", confirmAndFinish); + sendFromActiveProfile("stop"); +} + +function confirmAndFinish() { + let [win, doc] = getProfileInternals(gUid); + let [btn, msg] = getCleoControls(doc); + + ok(msg.style.display === "none", "Message is hidden"); + ok(!btn.hasAttribute("disabled"), "Button is enabled"); + + is(gPanel.document.querySelector("li#profile-1 > h1").textContent, + "Profile 1", "Profile 1 doesn't have a star next to it."); + is(gPanel.document.querySelector("li#profile-2 > h1").textContent, + "Profile 2", "Profile 2 doesn't have a star next to it."); + + tearDown(gTab, function onTearDown() { + gPanel = null; + gTab = null; + gUid = null; + }); +} \ No newline at end of file diff --git a/browser/devtools/profiler/test/browser_profiler_run.js b/browser/devtools/profiler/test/browser_profiler_run.js index 136864224279..87eb99458b9f 100644 --- a/browser/devtools/profiler/test/browser_profiler_run.js +++ b/browser/devtools/profiler/test/browser_profiler_run.js @@ -33,13 +33,6 @@ function attemptTearDown() { }); } -function getProfileInternals() { - let win = gPanel.activeProfile.iframe.contentWindow; - let doc = win.document; - - return [win, doc]; -} - function testUI() { ok(gPanel, "Profiler panel exists"); ok(gPanel.activeProfile, "Active profile exists"); diff --git a/browser/devtools/profiler/test/head.js b/browser/devtools/profiler/test/head.js index f2c317c61e2b..a24f45faac8b 100644 --- a/browser/devtools/profiler/test/head.js +++ b/browser/devtools/profiler/test/head.js @@ -20,6 +20,14 @@ registerCleanupFunction(function () { DebuggerServer.destroy(); }); +function getProfileInternals(uid) { + let profile = (uid != null) ? gPanel.profiles.get(uid) : gPanel.activeProfile; + let win = profile.iframe.contentWindow; + let doc = win.document; + + return [win, doc]; +} + function loadTab(url, callback) { let tab = gBrowser.addTab(); gBrowser.selectedTab = tab; diff --git a/browser/locales/en-US/chrome/browser/devtools/profiler.properties b/browser/locales/en-US/chrome/browser/devtools/profiler.properties index 1d27680209fa..05548d47e1d5 100644 --- a/browser/locales/en-US/chrome/browser/devtools/profiler.properties +++ b/browser/locales/en-US/chrome/browser/devtools/profiler.properties @@ -76,4 +76,9 @@ profiler.stop=Stop # LOCALIZATION NOTE (profiler.loading) # This string is displayed above the progress bar when the profiler # is busy loading and parsing the report. -profiler.loading=Loading profile… \ No newline at end of file +profiler.loading=Loading profile… + +# LOCALIZATION NOTE (profiler.alreadyRunning) +# This string is displayed in the profiler whenever there is already +# another running profile. Users can run only one profile at a time. +profiler.alreadyRunning=Profiler is already running. If you want to run this profile stop Profile %S first. \ No newline at end of file