From 2a72513df7afc02f20bd3e7ff04131d4aa948d59 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Wed, 2 Jul 2014 23:52:00 +0200 Subject: [PATCH] Bug 966895 - [rule view] Adding new rules to the current selection in the CSS rule-view r=harth --- browser/devtools/styleinspector/rule-view.js | 50 ++++- .../devtools/styleinspector/test/browser.ini | 2 + .../test/browser_ruleview_add-rule_01.js | 89 +++++++++ .../test/browser_ruleview_add-rule_02.js | 78 ++++++++ toolkit/devtools/server/actors/root.js | 5 +- toolkit/devtools/server/actors/styles.js | 183 +++++++++++++----- .../global/devtools/styleinspector.properties | 8 + 7 files changed, 364 insertions(+), 51 deletions(-) create mode 100644 browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js create mode 100644 browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 2f3e6eb6c14..c4457382590 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -442,7 +442,7 @@ Rule.prototype = { return this._title; } this._title = CssLogic.shortSource(this.sheet); - if (this.domRule.type !== ELEMENT_STYLE) { + if (this.domRule.type !== ELEMENT_STYLE && this.ruleLine > 0) { this._title += ":" + this.ruleLine; } @@ -1076,6 +1076,7 @@ function CssRuleView(aInspector, aDoc, aStore, aPageStyle) { this._buildContextMenu = this._buildContextMenu.bind(this); this._contextMenuUpdate = this._contextMenuUpdate.bind(this); + this._onAddRule = this._onAddRule.bind(this); this._onSelectAll = this._onSelectAll.bind(this); this._onCopy = this._onCopy.bind(this); this._onCopyColor = this._onCopyColor.bind(this); @@ -1125,6 +1126,11 @@ CssRuleView.prototype = { this._contextmenu.addEventListener("popupshowing", this._contextMenuUpdate); this._contextmenu.id = "rule-view-context-menu"; + this.menuitemAddRule = createMenuItem(this._contextmenu, { + label: "ruleView.contextmenu.addRule", + accesskey: "ruleView.contextmenu.addRule.accessKey", + command: this._onAddRule + }); this.menuitemSelectAll = createMenuItem(this._contextmenu, { label: "ruleView.contextmenu.selectAll", accesskey: "ruleView.contextmenu.selectAll.accessKey", @@ -1140,7 +1146,7 @@ CssRuleView.prototype = { accesskey: "ruleView.contextmenu.copyColor.accessKey", command: this._onCopyColor }); - this.menuitemSources= createMenuItem(this._contextmenu, { + this.menuitemSources = createMenuItem(this._contextmenu, { label: "ruleView.contextmenu.showOrigSources", accesskey: "ruleView.contextmenu.showOrigSources.accessKey", command: this._onToggleOrigSources @@ -1354,6 +1360,43 @@ CssRuleView.prototype = { Services.prefs.setBoolPref(PREF_ORIG_SOURCES, !isEnabled); }, + /** + * Add a new rule to the current element. + */ + _onAddRule: function() { + let elementStyle = this._elementStyle; + let element = elementStyle.element; + let rules = elementStyle.rules; + let client = this.inspector.toolbox._target.client; + + if (!client.traits.addNewRule) { + return; + } + + this.pageStyle.addNewRule(element).then(options => { + let newRule = new Rule(elementStyle, options); + elementStyle.rules.push(newRule); + let editor = new RuleEditor(this, newRule); + + // Insert the new rule editor after the inline element rule + if (rules.length <= 1) { + this.element.appendChild(editor.element); + } else { + for (let rule of rules) { + if (rule.selectorText === "element") { + let referenceElement = rule.editor.element.nextSibling; + this.element.insertBefore(editor.element, referenceElement); + break; + } + } + } + + // Focus and make the new rule's selector editable + editor.selectorText.click(); + elementStyle._changed(); + }); + }, + setPageStyle: function(aPageStyle) { this.pageStyle = aPageStyle; }, @@ -1852,6 +1895,9 @@ RuleEditor.prototype = { } } else { sourceLabel.setAttribute("value", this.rule.title); + if (this.rule.ruleLine == -1 && this.rule.domRule.parentStyleSheet) { + sourceLabel.parentNode.setAttribute("unselectable", "true"); + } } let showOrig = Services.prefs.getBoolPref(PREF_ORIG_SOURCES); diff --git a/browser/devtools/styleinspector/test/browser.ini b/browser/devtools/styleinspector/test/browser.ini index 363787de01e..1f5be910337 100644 --- a/browser/devtools/styleinspector/test/browser.ini +++ b/browser/devtools/styleinspector/test/browser.ini @@ -42,6 +42,8 @@ support-files = [browser_ruleview_add-property-cancel_03.js] [browser_ruleview_add-property_01.js] [browser_ruleview_add-property_02.js] +[browser_ruleview_add-rule_01.js] +[browser_ruleview_add-rule_02.js] [browser_ruleview_colorpicker-and-image-tooltip_01.js] [browser_ruleview_colorpicker-and-image-tooltip_02.js] [browser_ruleview_colorpicker-appears-on-swatch-click.js] diff --git a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js new file mode 100644 index 00000000000..5c572c4bfb6 --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js @@ -0,0 +1,89 @@ +/* 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"; + +// Tests the behaviour of adding a new rule to the rule view and the +// various inplace-editor behaviours in the new rule editor + +let PAGE_CONTENT = [ + '', + '
Styled Node
', + 'This is a span', + '

Empty

' +].join("\n"); + +const TEST_DATA = [ + { node: "#testid", expected: "#testid" }, + { node: ".testclass2", expected: ".testclass2" }, + { node: "p", expected: "p" } +]; + +let test = asyncTest(function*() { + yield addTab("data:text/html;charset=utf-8,test rule view add rule"); + + info("Creating the test document"); + content.document.body.innerHTML = PAGE_CONTENT; + + info("Opening the rule-view"); + let {toolbox, inspector, view} = yield openRuleView(); + + info("Iterating over the test data"); + for (let data of TEST_DATA) { + yield runTestData(inspector, view, data); + } +}); + +function* runTestData(inspector, view, data) { + let {node, expected} = data; + info("Selecting the test element"); + yield selectNode(node, inspector); + + info("Waiting for context menu to be shown"); + let onPopup = once(view._contextmenu, "popupshown"); + let win = view.doc.defaultView; + + EventUtils.synthesizeMouseAtCenter(view.element, + {button: 2, type: "contextmenu"}, win); + yield onPopup; + + ok(!view.menuitemAddRule.hidden, "Add rule is visible"); + + info("Waiting for rule view to change"); + let onRuleViewChanged = once(view.element, "CssRuleViewChanged"); + + info("Adding the new rule"); + view.menuitemAddRule.click(); + yield onRuleViewChanged; + view._contextmenu.hidePopup(); + + yield testNewRule(view, expected, 1); + + info("Resetting page content"); + content.document.body.innerHTML = PAGE_CONTENT; +} + +function* testNewRule(view, expected, index) { + let idRuleEditor = getRuleViewRuleEditor(view, index); + let editor = idRuleEditor.selectorText.ownerDocument.activeElement; + is(editor.value, expected, + "Selector editor value is as expected: " + expected); + + info("Entering the escape key"); + EventUtils.synthesizeKey("VK_ESCAPE", {}); + + is(idRuleEditor.selectorText.textContent, expected, + "Selector text value is as expected: " + expected); + + info("Adding new properties to new rule: " + expected) + idRuleEditor.addProperty("font-weight", "bold", ""); + let textProps = idRuleEditor.rule.textProps; + let lastRule = textProps[textProps.length - 1]; + is(lastRule.name, "font-weight", "Last rule name is font-weight"); + is(lastRule.value, "bold", "Last rule value is bold"); +} diff --git a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js new file mode 100644 index 00000000000..ff0c8698b12 --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js @@ -0,0 +1,78 @@ +/* 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"; + +// Tests the behaviour of adding a new rule to the rule view and editing +// its selector + +let PAGE_CONTENT = [ + '', + '

Styled Node
', + 'This is a span' +].join("\n"); + +let test = asyncTest(function*() { + yield addTab("data:text/html;charset=utf-8,test rule view add rule"); + + info("Creating the test document"); + content.document.body.innerHTML = PAGE_CONTENT; + + info("Opening the rule-view"); + let {toolbox, inspector, view} = yield openRuleView(); + + info("Selecting the test element"); + yield selectNode("#testid", inspector); + + info("Waiting for context menu to be shown"); + let onPopup = once(view._contextmenu, "popupshown"); + let win = view.doc.defaultView; + + EventUtils.synthesizeMouseAtCenter(view.element, + {button: 2, type: "contextmenu"}, win); + yield onPopup; + + ok(!view.menuitemAddRule.hidden, "Add rule is visible"); + + info("Waiting for rule view to change"); + let onRuleViewChanged = once(view.element, "CssRuleViewChanged"); + + info("Adding the new rule"); + view.menuitemAddRule.click(); + yield onRuleViewChanged; + view._contextmenu.hidePopup(); + + yield testEditSelector(view, "span"); + + info("Selecting the modified element"); + yield selectNode("span", inspector); + yield checkModifiedElement(view, "span"); +}); + +function* testEditSelector(view, name) { + info("Test editing existing selector field"); + let idRuleEditor = getRuleViewRuleEditor(view, 1); + let editor = idRuleEditor.selectorText.ownerDocument.activeElement; + + info("Entering a new selector name and committing"); + editor.value = name; + + info("Waiting for rule view to refresh"); + let onRuleViewRefresh = once(view.element, "CssRuleViewRefreshed"); + + info("Entering the commit key"); + EventUtils.synthesizeKey("VK_RETURN", {}); + yield onRuleViewRefresh; + + is(view._elementStyle.rules.length, 2, "Should have 2 rules."); +} + +function* checkModifiedElement(view, name) { + is(view._elementStyle.rules.length, 2, "Should have 2 rules."); + ok(getRuleViewRule(view, name), "Rule with " + name + " selector exists."); +} diff --git a/toolkit/devtools/server/actors/root.js b/toolkit/devtools/server/actors/root.js index e40bc43e424..c2839a35425 100644 --- a/toolkit/devtools/server/actors/root.js +++ b/toolkit/devtools/server/actors/root.js @@ -124,7 +124,10 @@ RootActor.prototype = { bulk: true, // Whether the style rule actor implements the modifySelector method // that modifies the rule's selector - selectorEditable: true + selectorEditable: true, + // Whether the page style actor implements the addNewRule method that + // adds new rules to the page + addNewRule: true }, /** diff --git a/toolkit/devtools/server/actors/styles.js b/toolkit/devtools/server/actors/styles.js index fe6e6ee84de..1509a9e0a51 100644 --- a/toolkit/devtools/server/actors/styles.js +++ b/toolkit/devtools/server/actors/styles.js @@ -29,6 +29,9 @@ exports.PSEUDO_ELEMENTS = PSEUDO_ELEMENTS; // Predeclare the domnode actor type for use in requests. types.addActorType("domnode"); +// Predeclare the domstylerule actor type +types.addActorType("domstylerule"); + /** * DOM Nodes returned by the style actor will be owned by the DOM walker * for the connection. @@ -52,6 +55,12 @@ types.addDictType("matchedselector", { status: "number" }); +types.addDictType("appliedStylesReturn", { + entries: "array:appliedstyle", + rules: "array:domstylerule", + sheets: "array:stylesheet" +}); + /** * The PageStyle actor lets the client look at the styles on a page, as * they are applied to a given node. @@ -79,6 +88,9 @@ var PageStyleActor = protocol.ActorClass({ // Stores the association of DOM objects -> actors this.refMap = new Map; + + this.onFrameUnload = this.onFrameUnload.bind(this); + events.on(this.inspector.tabActor, "will-navigate", this.onFrameUnload); }, get conn() this.inspector.conn, @@ -279,7 +291,6 @@ var PageStyleActor = protocol.ActorClass({ /** * Get the set of styles that apply to a given node. * @param NodeActor node - * @param string property * @param object options * `filter`: A string filter that affects the "matched" handling. * 'user': Include properties from user style sheets. @@ -291,46 +302,8 @@ var PageStyleActor = protocol.ActorClass({ */ getApplied: method(function(node, options) { let entries = []; - this.addElementRules(node.rawNode, undefined, options, entries); - - if (options.inherited) { - let parent = this.walker.parentNode(node); - while (parent && parent.rawNode.nodeType != Ci.nsIDOMNode.DOCUMENT_NODE) { - this.addElementRules(parent.rawNode, parent, options, entries); - parent = this.walker.parentNode(parent); - } - } - - if (options.matchedSelectors) { - for (let entry of entries) { - if (entry.rule.type === ELEMENT_STYLE) { - continue; - } - - let domRule = entry.rule.rawRule; - let selectors = CssLogic.getSelectors(domRule); - let element = entry.inherited ? entry.inherited.rawNode : node.rawNode; - entry.matchedSelectors = []; - for (let i = 0; i < selectors.length; i++) { - if (DOMUtils.selectorMatchesElement(element, domRule, i)) { - entry.matchedSelectors.push(selectors[i]); - } - } - - } - } - - let rules = new Set; - let sheets = new Set; - entries.forEach(entry => rules.add(entry.rule)); - this.expandSets(rules, sheets); - - return { - entries: entries, - rules: [...rules], - sheets: [...sheets] - } + return this.getAppliedProps(node, entries, options); }, { request: { node: Arg(0, "domnode"), @@ -338,11 +311,7 @@ var PageStyleActor = protocol.ActorClass({ matchedSelectors: Option(1, "boolean"), filter: Option(1, "string") }, - response: RetVal(types.addDictType("appliedStylesReturn", { - entries: "array:appliedstyle", - rules: "array:domstylerule", - sheets: "array:stylesheet" - })) + response: RetVal("appliedStylesReturn") }), _hasInheritedProps: function(style) { @@ -414,6 +383,66 @@ var PageStyleActor = protocol.ActorClass({ } }, + /** + * Helper function for getApplied and addNewRule that fetches a set of + * style properties that apply to the given node and associated rules + * @param NodeActor node + * @param object options + * `filter`: A string filter that affects the "matched" handling. + * 'user': Include properties from user style sheets. + * 'ua': Include properties from user and user-agent sheets. + * Default value is 'ua' + * `inherited`: Include styles inherited from parent nodes. + * `matchedSeletors`: Include an array of specific selectors that + * caused this rule to match its node. + * @param array entries + * List of appliedstyle objects that lists the rules that apply to the + * node. If adding a new rule to the stylesheet, only the new rule entry + * is provided and only the style properties that apply to the new + * rule is fetched. + * @returns Object containing the list of rule entries, rule actors and + * stylesheet actors that applies to the given node and its associated + * rules. + */ + getAppliedProps: function(node, entries, options) { + if (options.inherited) { + let parent = this.walker.parentNode(node); + while (parent && parent.rawNode.nodeType != Ci.nsIDOMNode.DOCUMENT_NODE) { + this.addElementRules(parent.rawNode, parent, options, entries); + parent = this.walker.parentNode(parent); + } + } + + if (options.matchedSelectors) { + for (let entry of entries) { + if (entry.rule.type === ELEMENT_STYLE) { + continue; + } + + let domRule = entry.rule.rawRule; + let selectors = CssLogic.getSelectors(domRule); + let element = entry.inherited ? entry.inherited.rawNode : node.rawNode; + entry.matchedSelectors = []; + for (let i = 0; i < selectors.length; i++) { + if (DOMUtils.selectorMatchesElement(element, domRule, i)) { + entry.matchedSelectors.push(selectors[i]); + } + } + } + } + + let rules = new Set; + let sheets = new Set; + entries.forEach(entry => rules.add(entry.rule)); + this.expandSets(rules, sheets); + + return { + entries: entries, + rules: [...rules], + sheets: [...sheets] + } + }, + /** * Expand Sets of rules and sheets to include all parent rules and sheets. */ @@ -516,6 +545,59 @@ var PageStyleActor = protocol.ActorClass({ return margins; }, + /** + * On page navigation, tidy up remaining objects. + */ + onFrameUnload: function() { + this._styleElement = null; + }, + + /** + * Helper function to addNewRule to construct a new style tag in the document. + * @returns DOMElement of the style tag + */ + get styleElement() { + if (!this._styleElement) { + let document = this.inspector.window.document; + let style = document.createElement("style"); + style.setAttribute("type", "text/css"); + document.head.appendChild(style); + this._styleElement = style; + } + + return this._styleElement; + }, + + /** + * Adds a new rule, and returns the new StyleRuleActor. + * @param NodeActor node + * @returns StyleRuleActor of the new rule + */ + addNewRule: method(function(node) { + let style = this.styleElement; + let sheet = style.sheet; + let rawNode = node.rawNode; + + let selector; + if (rawNode.id) { + selector = "#" + rawNode.id; + } else if (rawNode.className) { + selector = "." + rawNode.className; + } else { + selector = rawNode.tagName.toLowerCase(); + } + + let index = sheet.insertRule(selector +" {}", sheet.cssRules.length); + let ruleActor = this._styleRef(sheet.cssRules[index]); + return this.getAppliedProps(node, [{ rule: ruleActor }], + { matchedSelectors: true }); + }, { + request: { + node: Arg(0, "domnode") + }, + response: RetVal("appliedStylesReturn") + }), + }); exports.PageStyleActor = PageStyleActor; @@ -550,12 +632,17 @@ var PageStyleFront = protocol.FrontClass(PageStyleActor, { }); }, { impl: "_getApplied" + }), + + addNewRule: protocol.custom(function(node) { + return this._addNewRule(node).then(ret => { + return ret.entries[0]; + }); + }, { + impl: "_addNewRule" }) }); -// Predeclare the domstylerule actor type -types.addActorType("domstylerule"); - /** * An actor that represents a CSS style object on the protocol. * diff --git a/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties b/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties index 91d232d6aec..3dce179519c 100644 --- a/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties +++ b/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties @@ -104,6 +104,14 @@ ruleView.contextmenu.showCSSSources=Show CSS sources # the rule view context menu "Show CSS sources" entry. ruleView.contextmenu.showCSSSources.accessKey=S +# LOCALIZATION NOTE (ruleView.contextmenu.addRule): Text displayed in the +# rule view context menu for adding a new rule to the element. +ruleView.contextmenu.addRule=Add rule + +# LOCALIZATION NOTE (ruleView.contextmenu.addRule.accessKey): Access key for +# the rule view context menu "Add rule" entry. +ruleView.contextmenu.addRule.accessKey=R + # LOCALIZATION NOTE (computedView.contextmenu.selectAll): Text displayed in the # computed view context menu. computedView.contextmenu.selectAll=Select all