diff --git a/devtools/client/styleinspector/rule-view.js b/devtools/client/styleinspector/rule-view.js index 14ed5aff2c4f..72c66be456d1 100644 --- a/devtools/client/styleinspector/rule-view.js +++ b/devtools/client/styleinspector/rule-view.js @@ -1317,6 +1317,7 @@ function CssRuleView(inspector, document, store, pageStyle) { this._outputParser = new OutputParser(document); + this._onKeydown = this._onKeydown.bind(this); this._onKeypress = this._onKeypress.bind(this); this._onAddRule = this._onAddRule.bind(this); this._onContextMenu = this._onContextMenu.bind(this); @@ -1342,6 +1343,7 @@ function CssRuleView(inspector, document, store, pageStyle) { this.searchClearButton.hidden = true; + this.styleDocument.addEventListener("keydown", this._onKeydown); this.styleDocument.addEventListener("keypress", this._onKeypress); this.element.addEventListener("copy", this._onCopy); this.element.addEventListener("contextmenu", this._onContextMenu); @@ -1739,9 +1741,7 @@ CssRuleView.prototype = { // Reselect the currently selected element let refreshOnPrefs = [PREF_UA_STYLES, PREF_DEFAULT_COLOR_UNIT]; if (refreshOnPrefs.indexOf(pref) > -1) { - let element = this._viewedElement; - this._viewedElement = null; - this.selectElement(element); + this.selectElement(this._viewedElement, true); } }, @@ -1912,6 +1912,8 @@ CssRuleView.prototype = { this.highlighters.destroy(); // Remove bound listeners + this.styleDocument.removeEventListener("keydown", this._onKeydown); + this.styleDocument.removeEventListener("keypress", this._onKeypress); this.element.removeEventListener("copy", this._onCopy); this.element.removeEventListener("contextmenu", this._onContextMenu); this.addRuleButton.removeEventListener("click", this._onAddRule); @@ -1949,14 +1951,35 @@ CssRuleView.prototype = { this.popup.destroy(); }, + + + /** + * Mark the view as selecting an element, disabling all interaction, and + * visually clearing the view after a few milliseconds to avoid confusion + * about which element's styles the rule view shows. + */ + _startSelectingElement: function() { + this.element.classList.add("non-interactive"); + }, + + /** + * Mark the view as no longer selecting an element, re-enabling interaction. + */ + _stopSelectingElement: function() { + this.element.classList.remove("non-interactive"); + }, + /** * Update the view with a new selected element. * * @param {NodeActor} element * The node whose style rules we'll inspect. + * @param {Boolean} allowRefresh + * Update the view even if the element is the same as last time. */ - selectElement: function(element) { - if (this._viewedElement === element) { + selectElement: function(element, allowRefresh=false) { + let refresh = (this._viewedElement === element); + if (refresh && !allowRefresh) { return promise.resolve(undefined); } @@ -1964,32 +1987,47 @@ CssRuleView.prototype = { this.popup.hidePopup(); } - this.clear(); - this.clearPseudoClassPanel(); - + this.clear(false); this._viewedElement = element; + + this.clearPseudoClassPanel(); this.refreshAddRuleButtonState(); if (!this._viewedElement) { + this._stopSelectingElement(); + this._clearRules(); this._showEmpty(); this.refreshPseudoClassPanel(); return promise.resolve(undefined); } - this._elementStyle = new ElementStyle(element, this.store, + let elementStyle = new ElementStyle(element, this.store, this.pageStyle, this.showUserAgentStyles); + this._elementStyle = elementStyle; + + this._startSelectingElement(); return this._elementStyle.init().then(() => { - if (this._viewedElement === element) { + if (this._elementStyle === elementStyle) { return this._populate(); } }).then(() => { - if (this._viewedElement === element) { + if (this._elementStyle === elementStyle) { + if (!refresh) { + this.element.scrollTop = 0; + } + this._stopSelectingElement(); this._elementStyle.onChanged = () => { this._changed(); }; } - }).then(null, console.error); + }).then(null, e => { + if (this._elementStyle === elementStyle) { + this._stopSelectingElement(); + this._clearRules(); + } + console.error(e); + }); }, /** @@ -2010,7 +2048,7 @@ CssRuleView.prototype = { } return promise.all(promises).then(() => { - return this._populate(true); + return this._populate(); }); }, @@ -2053,16 +2091,14 @@ CssRuleView.prototype = { } }, - _populate: function(clearRules = false) { + _populate: function() { let elementStyle = this._elementStyle; return this._elementStyle.populate().then(() => { if (this._elementStyle !== elementStyle || this.isDestroyed) { return; } - if (clearRules) { - this._clearRules(); - } + this._clearRules(); this._createEditors(); this.refreshPseudoClassPanel(); @@ -2090,18 +2126,18 @@ CssRuleView.prototype = { * Clear the rules. */ _clearRules: function() { - while (this.element.hasChildNodes()) { - this.element.removeChild(this.element.lastChild); - } + this.element.innerHTML = ""; }, /** * Clear the rule view. */ - clear: function() { + clear: function(clearDom=true) { this.lastSelectorIcon = null; - this._clearRules(); + if (clearDom) { + this._clearRules(); + } this._viewedElement = null; if (this._elementStyle) { @@ -2583,6 +2619,16 @@ CssRuleView.prototype = { this.inspector.togglePseudoClass(target.value); }, + /** + * Handle the keydown event in the rule view. + */ + _onKeydown: function(event) { + if (this.element.classList.contains("non-interactive") && + (event.code === "Enter" || event.code === " ")) { + event.preventDefault(); + } + }, + /** * Handle the keypress event in the rule view. */ diff --git a/devtools/client/styleinspector/ruleview.css b/devtools/client/styleinspector/ruleview.css index 545d883549d6..2ea3903ad8b6 100644 --- a/devtools/client/styleinspector/ruleview.css +++ b/devtools/client/styleinspector/ruleview.css @@ -24,6 +24,12 @@ body { flex: 1; } +#ruleview-container.non-interactive { + pointer-events: none; + visibility: collapse; + transition: visibility 0.25s; +} + .devtools-toolbar { width: 100%; display: flex; diff --git a/devtools/client/styleinspector/style-inspector-overlays.js b/devtools/client/styleinspector/style-inspector-overlays.js index 317feeb95bb9..9b408f0ab435 100644 --- a/devtools/client/styleinspector/style-inspector-overlays.js +++ b/devtools/client/styleinspector/style-inspector-overlays.js @@ -55,7 +55,6 @@ function HighlightersOverlay(view) { this._onMouseMove = this._onMouseMove.bind(this); this._onMouseLeave = this._onMouseLeave.bind(this); - this.promises = {}; this.highlighters = {}; // Only initialize the overlay if at least one of the highlighter types is @@ -176,20 +175,21 @@ HighlightersOverlay.prototype = { * Hide the currently shown highlighter */ _hideCurrent: function() { - if (this.highlighterShown) { - this._getHighlighter(this.highlighterShown).then(highlighter => { - // For some reason, the call to highlighter.hide doesn't always return a - // promise. This causes some tests to fail when trying to install a - // rejection handler on the result of the call. To avoid this, check - // whether the result is truthy before installing the handler. - let promise = highlighter.hide(); - if (promise) { - promise.then(null, Cu.reportError); - } - this.highlighterShown = null; - this.emit("highlighter-hidden"); - }); + if (!this.highlighterShown || !this.highlighters[this.highlighterShown]) { + return; } + + // For some reason, the call to highlighter.hide doesn't always return a + // promise. This causes some tests to fail when trying to install a + // rejection handler on the result of the call. To avoid this, check + // whether the result is truthy before installing the handler. + let promise = this.highlighters[this.highlighterShown].hide(); + if (promise) { + promise.then(null, e => console.error(e)); + } + + this.highlighterShown = null; + this.emit("highlighter-hidden"); }, /** @@ -200,11 +200,11 @@ HighlightersOverlay.prototype = { _getHighlighter: function(type) { let utils = this.highlighterUtils; - if (this.promises[type]) { - return this.promises[type]; + if (this.highlighters[type]) { + return promise.resolve(this.highlighters[type]); } - return this.promises[type] = utils.getHighlighterByType(type).then(highlighter => { + return utils.getHighlighterByType(type).then(highlighter => { this.highlighters[type] = highlighter; return highlighter; }); @@ -224,7 +224,6 @@ HighlightersOverlay.prototype = { } } - this.promises = null; this.view = null; this.highlighterUtils = null; diff --git a/devtools/client/styleinspector/test/browser.ini b/devtools/client/styleinspector/test/browser.ini index 7e052e42b65f..c465897d2354 100644 --- a/devtools/client/styleinspector/test/browser.ini +++ b/devtools/client/styleinspector/test/browser.ini @@ -150,6 +150,7 @@ skip-if = (os == "win" && debug) || e10s # bug 963492: win. bug 1040653: e10s. [browser_ruleview_pseudo-element_02.js] skip-if = e10s # Bug 1090340 [browser_ruleview_pseudo_lock_options.js] +[browser_ruleview_refresh-no-flicker.js] [browser_ruleview_refresh-on-attribute-change_01.js] [browser_ruleview_refresh-on-attribute-change_02.js] [browser_ruleview_refresh-on-style-change.js] diff --git a/devtools/client/styleinspector/test/browser_ruleview_refresh-no-flicker.js b/devtools/client/styleinspector/test/browser_ruleview_refresh-no-flicker.js new file mode 100644 index 000000000000..217ba43194c0 --- /dev/null +++ b/devtools/client/styleinspector/test/browser_ruleview_refresh-no-flicker.js @@ -0,0 +1,38 @@ +/* vim: set ft=javascript ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that the rule view does not go blank while selecting a new node. + +const TESTCASE_URI = 'data:text/html;charset=utf-8,' + + '
Test div!
'; + +add_task(function*() { + yield addTab(TESTCASE_URI); + + info("Opening the rule view and selecting the test node"); + let {toolbox, inspector, view} = yield openRuleView(); + let testdiv = yield getNodeFront("#testdiv", inspector); + yield selectNode(testdiv, inspector); + + let htmlBefore = view.element.innerHTML; + ok(htmlBefore.indexOf("font-size") > -1, + "The rule view should contain a font-size property."); + + // Do the selectNode call manually, because otherwise it's hard to guarantee + // that we can make the below checks at a reasonable time. + info("refreshing the node"); + let p = view.selectElement(testdiv, true); + is(view.element.innerHTML, htmlBefore, + "The rule view is unchanged during selection."); + ok(view.element.classList.contains("non-interactive"), + "The rule view is marked non-interactive."); + yield p; + + info("node refreshed"); + ok(!view.element.classList.contains("non-interactive"), + "The rule view is marked interactive again."); +}); + diff --git a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-02.js b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-02.js index b430e5741ba3..8e77cbc97b80 100644 --- a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-02.js +++ b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-02.js @@ -25,24 +25,17 @@ add_task(function*() { let hs = view.highlighters; ok(!hs.highlighters[TYPE], "No highlighter exists in the rule-view (1)"); - ok(!hs.promises[TYPE], - "No highlighter is being created in the rule-view (1)"); info("Faking a mousemove on a non-transform property"); let {valueSpan} = getRuleViewProperty(view, "body", "color"); hs._onMouseMove({target: valueSpan}); ok(!hs.highlighters[TYPE], "No highlighter exists in the rule-view (2)"); - ok(!hs.promises[TYPE], - "No highlighter is being created in the rule-view (2)"); info("Faking a mousemove on a transform property"); ({valueSpan} = getRuleViewProperty(view, "body", "transform")); let onHighlighterShown = hs.once("highlighter-shown"); hs._onMouseMove({target: valueSpan}); yield onHighlighterShown; - ok(hs.promises[TYPE], "The highlighter is being initialized"); - let h = yield hs.promises[TYPE]; - is(h, hs.highlighters[TYPE], "The initialized highlighter is the right one"); let onComputedViewReady = inspector.once("computed-view-refreshed"); let {view: cView} = yield openComputedView(); @@ -50,22 +43,15 @@ add_task(function*() { hs = cView.highlighters; ok(!hs.highlighters[TYPE], "No highlighter exists in the computed-view (1)"); - ok(!hs.promises[TYPE], - "No highlighter is being created in the computed-view (1)"); info("Faking a mousemove on a non-transform property"); ({valueSpan} = getComputedViewProperty(cView, "color")); hs._onMouseMove({target: valueSpan}); ok(!hs.highlighters[TYPE], "No highlighter exists in the computed-view (2)"); - ok(!hs.promises[TYPE], - "No highlighter is being created in the computed-view (2)"); info("Faking a mousemove on a transform property"); ({valueSpan} = getComputedViewProperty(cView, "transform")); onHighlighterShown = hs.once("highlighter-shown"); hs._onMouseMove({target: valueSpan}); yield onHighlighterShown; - ok(hs.promises[TYPE], "The highlighter is being initialized"); - h = yield hs.promises[TYPE]; - is(h, hs.highlighters[TYPE], "The initialized highlighter is the right one"); }); diff --git a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-03.js b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-03.js index 72b687ec0bbb..4105305e5a3e 100644 --- a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-03.js +++ b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-03.js @@ -44,12 +44,13 @@ add_task(function*() { this.nodeFront = null; this.isShown = false; return promise.resolve(); - } + }, + finalize: function() {} }; // Inject the mock highlighter in the rule-view let hs = view.highlighters; - hs.promises[TYPE] = promise.resolve(HighlighterFront); + hs.highlighters[TYPE] = HighlighterFront; let {valueSpan} = getRuleViewProperty(view, "body", "transform"); diff --git a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-04.js b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-04.js index 0e3d7ef8a5ef..d8e3f793afce 100644 --- a/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-04.js +++ b/devtools/client/styleinspector/test/browser_styleinspector_transform-highlighter-04.js @@ -39,7 +39,6 @@ add_task(function*() { hs._onMouseMove({target: valueSpan}); ok(!hs.highlighters[TYPE], "No highlighter was created for the overriden property"); - ok(!hs.promises[TYPE], "And no highlighter is being initialized either"); info("Disabling the applied property"); let classRuleEditor = getRuleViewRuleEditor(view, 1); @@ -52,14 +51,10 @@ add_task(function*() { hs._onMouseMove({target: valueSpan}); ok(!hs.highlighters[TYPE], "No highlighter was created for the disabled property"); - ok(!hs.promises[TYPE], "And no highlighter is being initialized either"); info("Faking a mousemove on the now unoverriden property"); ({valueSpan} = getRuleViewProperty(view, "div", "transform")); let onHighlighterShown = hs.once("highlighter-shown"); hs._onMouseMove({target: valueSpan}); yield onHighlighterShown; - ok(hs.promises[TYPE], "The highlighter is being initialized now"); - let h = yield hs.promises[TYPE]; - is(h, hs.highlighters[TYPE], "The initialized highlighter is the right one"); });