From 12ea5683594229407ae1852969b6417a59ae16e1 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Thu, 1 Mar 2018 13:13:33 +0100 Subject: DevTools - style editor - StyleSheetsActor should use the parent tabActor to retrieve the list of windows and react to new/removed windows https://github.com/MoonchildProductions/moebius/pull/124 --- devtools/client/styleeditor/StyleEditorUI.jsm | 109 +++++++++++++------ devtools/client/styleeditor/test/browser.ini | 1 + .../test/browser_styleeditor_add_stylesheet.js | 37 +++++++ .../styleeditor/test/browser_styleeditor_import.js | 2 +- devtools/server/actors/stylesheets.js | 120 +++++++++++++++------ devtools/shared/specs/stylesheets.js | 8 ++ 6 files changed, 212 insertions(+), 65 deletions(-) create mode 100644 devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js diff --git a/devtools/client/styleeditor/StyleEditorUI.jsm b/devtools/client/styleeditor/StyleEditorUI.jsm index cdb267669..b2735b3fc 100644 --- a/devtools/client/styleeditor/StyleEditorUI.jsm +++ b/devtools/client/styleeditor/StyleEditorUI.jsm @@ -72,10 +72,18 @@ function StyleEditorUI(debuggee, target, panelDoc, cssProperties) { this.editors = []; this.selectedEditor = null; this.savedLocations = {}; + this._seenSheets = new Map(); + + // Don't add any style sheets that might arrive via events, until + // the call to initialize. Style sheets can arrive from the server + // at any time, for example if a new style sheet was added, or if + // the style sheet actor was just created and is walking the style + // sheets for the first time. In any case, in |initialize| we're + // going to fetch the list of sheets anyway. + this._suppressAdd = true; this._onOptionsPopupShowing = this._onOptionsPopupShowing.bind(this); this._onOptionsPopupHiding = this._onOptionsPopupHiding.bind(this); - this._onStyleSheetCreated = this._onStyleSheetCreated.bind(this); this._onNewDocument = this._onNewDocument.bind(this); this._onMediaPrefChanged = this._onMediaPrefChanged.bind(this); this._updateMediaList = this._updateMediaList.bind(this); @@ -83,10 +91,13 @@ function StyleEditorUI(debuggee, target, panelDoc, cssProperties) { this._onError = this._onError.bind(this); this._updateOpenLinkItem = this._updateOpenLinkItem.bind(this); this._openLinkNewTab = this._openLinkNewTab.bind(this); + this._addStyleSheet = this._addStyleSheet.bind(this); this._prefObserver = new PrefObserver("devtools.styleeditor."); this._prefObserver.on(PREF_ORIG_SOURCES, this._onNewDocument); this._prefObserver.on(PREF_MEDIA_SIDEBAR, this._onMediaPrefChanged); + + this._debuggee.on("stylesheet-added", this._addStyleSheet); } this.StyleEditorUI = StyleEditorUI; @@ -165,7 +176,7 @@ StyleEditorUI.prototype = { this._view = new SplitView(viewRoot); wire(this._view.rootElement, ".style-editor-newButton", () =>{ - this._debuggee.addStyleSheet(null).then(this._onStyleSheetCreated); + this._debuggee.addStyleSheet(null); }); wire(this._view.rootElement, ".style-editor-importButton", ()=> { @@ -233,6 +244,7 @@ StyleEditorUI.prototype = { * StyleSheet object for new sheet */ _onNewDocument: function () { + this._suppressAdd = true; this._debuggee.getStyleSheets().then((styleSheets) => { return this._resetStyleSheetList(styleSheets); }).then(null, e => console.error(e)); @@ -246,6 +258,7 @@ StyleEditorUI.prototype = { */ _resetStyleSheetList: Task.async(function* (styleSheets) { this._clear(); + this._suppressAdd = false; for (let sheet of styleSheets) { try { @@ -288,6 +301,10 @@ StyleEditorUI.prototype = { this._view.removeAll(); this.selectedEditor = null; + // Here the keys are style sheet actors, and the values are + // promises that resolve to the sheet's editor. See |_addStyleSheet|. + this._seenSheets = new Map(); + this._suppressAdd = true; this._root.classList.add("loading"); }, @@ -298,46 +315,67 @@ StyleEditorUI.prototype = { * * @param {StyleSheetFront} styleSheet * Style sheet to add to style editor + * @param {Boolean} isNew + * True if this style sheet was created by a call to the + * style sheets actor's @see addStyleSheet method. + * @return {Promise} + * A promise that resolves to the style sheet's editor when the style sheet has + * been fully loaded. If the style sheet has a source map, and source mapping + * is enabled, then the promise resolves to null. */ - _addStyleSheet: Task.async(function* (styleSheet) { - let editor = yield this._addStyleSheetEditor(styleSheet); - - if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) { - return; + _addStyleSheet: function (styleSheet, isNew) { + if (this._suppressAdd) { + return null; } - let sources = yield styleSheet.getOriginalSources(); - if (sources && sources.length) { - let parentEditorName = editor.friendlyName; - this._removeStyleSheetEditor(editor); - - for (let source of sources) { - // set so the first sheet will be selected, even if it's a source - source.styleSheetIndex = styleSheet.styleSheetIndex; - source.relatedStyleSheet = styleSheet; - source.relatedEditorName = parentEditorName; - yield this._addStyleSheetEditor(source); - } + if (!this._seenSheets.has(styleSheet)) { + let promise = (async () => { + let editor = await this._addStyleSheetEditor(styleSheet, isNew); + + if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) { + return editor; + } + + let sources = await styleSheet.getOriginalSources(); + // A single generated sheet might map to multiple original + // sheets, so make editors for each of them. + if (sources && sources.length) { + let parentEditorName = editor.friendlyName; + this._removeStyleSheetEditor(editor); + editor = null; + + for (let source of sources) { + // set so the first sheet will be selected, even if it's a source + source.styleSheetIndex = styleSheet.styleSheetIndex; + source.relatedStyleSheet = styleSheet; + source.relatedEditorName = parentEditorName; + await this._addStyleSheetEditor(source); + } + } + + return editor; + })(); + this._seenSheets.set(styleSheet, promise); } - }), + return this._seenSheets.get(styleSheet); + }, /** * Add a new editor to the UI for a source. * * @param {StyleSheet} styleSheet * Object representing stylesheet - * @param {nsIfile} file - * Optional file object that sheet was imported from * @param {Boolean} isNew * Optional if stylesheet is a new sheet created by user * @return {Promise} that is resolved with the created StyleSheetEditor when * the editor is fully initialized or rejected on error. */ - _addStyleSheetEditor: Task.async(function* (styleSheet, file, isNew) { + _addStyleSheetEditor: Task.async(function* (styleSheet, isNew) { // recall location of saved file for this sheet after page reload + let file = null; let identifier = this.getStyleSheetIdentifier(styleSheet); let savedFile = this.savedLocations[identifier]; - if (savedFile && !file) { + if (savedFile) { file = savedFile; } @@ -388,8 +426,16 @@ StyleEditorUI.prototype = { NetUtil.readInputStreamToString(stream, stream.available()); stream.close(); + this._suppressAdd = true; this._debuggee.addStyleSheet(source).then((styleSheet) => { - this._onStyleSheetCreated(styleSheet, selectedFile); + this._suppressAdd = false; + this._addStyleSheet(styleSheet, true).then(editor => { + if (editor) { + editor.savedFile = selectedFile; + } + // Just for testing purposes. + this.emit("test:editor-updated", editor); + }); }); }); }; @@ -397,14 +443,6 @@ StyleEditorUI.prototype = { showFilePicker(file, false, parentWindow, onFileSelected); }, - /** - * When a new or imported stylesheet has been added to the document. - * Add an editor for it. - */ - _onStyleSheetCreated: function (styleSheet, file) { - this._addStyleSheetEditor(styleSheet, file, true); - }, - /** * Forward any error from a stylesheet. * @@ -1013,6 +1051,9 @@ StyleEditorUI.prototype = { this._clearStyleSheetEditors(); + this._seenSheets = null; + this._suppressAdd = false; + let sidebar = this._panelDoc.querySelector(".splitview-controller"); let sidebarWidth = sidebar.getAttribute("width"); Services.prefs.setIntPref(PREF_NAV_WIDTH, sidebarWidth); @@ -1025,5 +1066,7 @@ StyleEditorUI.prototype = { this._prefObserver.off(PREF_ORIG_SOURCES, this._onNewDocument); this._prefObserver.off(PREF_MEDIA_SIDEBAR, this._onMediaPrefChanged); this._prefObserver.destroy(); + + this._debuggee.off("stylesheet-added", this._addStyleSheet); } }; diff --git a/devtools/client/styleeditor/test/browser.ini b/devtools/client/styleeditor/test/browser.ini index 1a85546af..4a84d45e6 100644 --- a/devtools/client/styleeditor/test/browser.ini +++ b/devtools/client/styleeditor/test/browser.ini @@ -60,6 +60,7 @@ support-files = !/devtools/client/shared/test/test-actor-registry.js !/devtools/client/shared/test/test-actor.js +[browser_styleeditor_add_stylesheet.js] [browser_styleeditor_autocomplete.js] [browser_styleeditor_autocomplete-disabled.js] [browser_styleeditor_bom.js] diff --git a/devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js b/devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js new file mode 100644 index 000000000..d8315d212 --- /dev/null +++ b/devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js @@ -0,0 +1,37 @@ +/* vim: set 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 a newly-added style sheet shows up in the style editor. + +const TESTCASE_URI = TEST_BASE_HTTPS + "simple.html"; + +add_task(function* () { + let { ui } = yield openStyleEditorForURL(TESTCASE_URI); + + is(ui.editors.length, 2, "Two sheets present after load."); + + // We have to wait for the length to change, because we might still + // be seeing events from the initial open. + let added = new Promise(resolve => { + let handler = () => { + if (ui.editors.length === 3) { + ui.off("editor-added", handler); + resolve(); + } + }; + ui.on("editor-added", handler); + }); + + info("Adding a style sheet"); + yield ContentTask.spawn(gBrowser.selectedBrowser, null, () => { + let document = content.document; + const style = document.createElement("style"); + style.appendChild(document.createTextNode("div { background: #f06; }")); + document.head.appendChild(style); + }); + yield added; + + is(ui.editors.length, 3, "Three sheets present after new style sheet"); +}); diff --git a/devtools/client/styleeditor/test/browser_styleeditor_import.js b/devtools/client/styleeditor/test/browser_styleeditor_import.js index f31f72ce7..2f42317b9 100644 --- a/devtools/client/styleeditor/test/browser_styleeditor_import.js +++ b/devtools/client/styleeditor/test/browser_styleeditor_import.js @@ -18,7 +18,7 @@ const SOURCE = "body{background:red;}"; add_task(function* () { let { panel, ui } = yield openStyleEditorForURL(TESTCASE_URI); - let added = ui.once("editor-added"); + let added = ui.once("test:editor-updated"); importSheet(ui, panel.panelWindow); info("Waiting for editor to be added for the imported sheet."); diff --git a/devtools/server/actors/stylesheets.js b/devtools/server/actors/stylesheets.js index f20634e6c..7fcbca8c4 100644 --- a/devtools/server/actors/stylesheets.js +++ b/devtools/server/actors/stylesheets.js @@ -13,7 +13,6 @@ const events = require("sdk/event/core"); const protocol = require("devtools/shared/protocol"); const {LongStringActor} = require("devtools/server/actors/string"); const {fetch} = require("devtools/shared/DevToolsUtils"); -const {listenOnce} = require("devtools/shared/async-utils"); const {originalSourceSpec, mediaRuleSpec, styleSheetSpec, styleSheetsSpec} = require("devtools/shared/specs/stylesheets"); const {SourceMapConsumer} = require("source-map"); @@ -251,7 +250,7 @@ var StyleSheetActor = protocol.ActorClassWithSpec(styleSheetSpec, { }, destroy: function () { - if (this._transitionTimeout) { + if (this._transitionTimeout && this.window) { this.window.clearTimeout(this._transitionTimeout); removePseudoClassLock( this.document.documentElement, TRANSITION_PSEUDO_CLASS); @@ -801,6 +800,64 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { protocol.Actor.prototype.initialize.call(this, null); this.parentActor = tabActor; + + this._onNewStyleSheetActor = this._onNewStyleSheetActor.bind(this); + this._onSheetAdded = this._onSheetAdded.bind(this); + this._onWindowReady = this._onWindowReady.bind(this); + + events.on(this.parentActor, "stylesheet-added", this._onNewStyleSheetActor); + events.on(this.parentActor, "window-ready", this._onWindowReady); + + // We listen for StyleSheetApplicableStateChanged rather than + // StyleSheetAdded, because the latter will be sent before the + // rules are ready. Using the former (with a check to ensure that + // the sheet is enabled) ensures that the sheet is ready before we + // try to make an actor for it. + this.parentActor.chromeEventHandler + .addEventListener("StyleSheetApplicableStateChanged", this._onSheetAdded, true); + + // This is used when creating a new style sheet, so that we can + // pass the correct flag when emitting our stylesheet-added event. + // See addStyleSheet and _onNewStyleSheetActor for more details. + this._nextStyleSheetIsNew = false; + }, + + destroy: function () { + for (let win of this.parentActor.windows) { + // This flag only exists for devtools, so we are free to clear + // it when we're done. + win.document.styleSheetChangeEventsEnabled = false; + } + + events.off(this.parentActor, "stylesheet-added", this._onNewStyleSheetActor); + events.off(this.parentActor, "window-ready", this._onWindowReady); + + this.parentActor.chromeEventHandler.removeEventListener("StyleSheetAdded", + this._onSheetAdded, true); + + protocol.Actor.prototype.destroy.call(this); + }, + + /** + * Event handler that is called when a the tab actor emits window-ready. + * + * @param {Event} evt + * The triggering event. + */ + _onWindowReady: function (evt) { + this._addStyleSheets(evt.window); + }, + + /** + * Event handler that is called when a the tab actor emits stylesheet-added. + * + * @param {StyleSheetActor} actor + * The new style sheet actor. + */ + _onNewStyleSheetActor: function (actor) { + // Forward it to the client side. + events.emit(this, "stylesheet-added", actor, this._nextStyleSheetIsNew); + this._nextStyleSheetIsNew = false; }, /** @@ -808,23 +865,11 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { * all the style sheets in this document. */ getStyleSheets: Task.async(function* () { - // Iframe document can change during load (bug 1171919). Track their windows - // instead. - let windows = [this.window]; let actors = []; - for (let win of windows) { + for (let win of this.parentActor.windows) { let sheets = yield this._addStyleSheets(win); actors = actors.concat(sheets); - - // Recursively handle style sheets of the documents in iframes. - for (let iframe of win.document.querySelectorAll("iframe, browser, frame")) { - if (iframe.contentDocument && iframe.contentWindow) { - // Sometimes, iframes don't have any document, like the - // one that are over deeply nested (bug 285395) - windows.push(iframe.contentWindow); - } - } } return actors; }), @@ -832,15 +877,13 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { /** * Check if we should be showing this stylesheet. * - * @param {Document} doc - * Document for which we're checking * @param {DOMCSSStyleSheet} sheet * Stylesheet we're interested in * * @return boolean * Whether the stylesheet should be listed. */ - _shouldListSheet: function (doc, sheet) { + _shouldListSheet: function (sheet) { // Special case about:PreferenceStyleSheet, as it is generated on the // fly and the URI is not registered with the about: handler. // https://bugzilla.mozilla.org/show_bug.cgi?id=935803#c37 @@ -851,6 +894,22 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { return true; }, + /** + * Event handler that is called when a new style sheet is added to + * a document. In particular, StyleSheetApplicableStateChanged is + * listened for, because StyleSheetAdded is sent too early, before + * the rules are ready. + * + * @param {Event} evt + * The triggering event. + */ + _onSheetAdded: function (evt) { + let sheet = evt.stylesheet; + if (this._shouldListSheet(sheet)) { + this.parentActor.createStyleSheetActor(sheet); + } + }, + /** * Add all the stylesheets for the document in this window to the map and * create an actor for each one if not already created. @@ -865,24 +924,16 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { { return Task.spawn(function* () { let doc = win.document; - // readyState can be uninitialized if an iframe has just been created but - // it has not started to load yet. - if (doc.readyState === "loading" || doc.readyState === "uninitialized") { - // Wait for the document to load first. - yield listenOnce(win, "DOMContentLoaded", true); - - // Make sure we have the actual document for this window. If the - // readyState was initially uninitialized, the initial dummy document - // was replaced with the actual document (bug 1171919). - doc = win.document; - } + // We have to set this flag in order to get the + // StyleSheetApplicableStateChanged events. See Document.webidl. + doc.styleSheetChangeEventsEnabled = true; let isChrome = Services.scriptSecurityManager.isSystemPrincipal(doc.nodePrincipal); let styleSheets = isChrome ? DOMUtils.getAllStyleSheets(doc) : doc.styleSheets; let actors = []; for (let i = 0; i < styleSheets.length; i++) { let sheet = styleSheets[i]; - if (!this._shouldListSheet(doc, sheet)) { + if (!this._shouldListSheet(sheet)) { continue; } @@ -917,7 +968,7 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { if (rule.type == Ci.nsIDOMCSSRule.IMPORT_RULE) { // Associated styleSheet may be null if it has already been seen due // to duplicate @imports for the same URL. - if (!rule.styleSheet || !this._shouldListSheet(doc, rule.styleSheet)) { + if (!rule.styleSheet || !this._shouldListSheet(rule.styleSheet)) { continue; } let actor = this.parentActor.createStyleSheetActor(rule.styleSheet); @@ -948,6 +999,13 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, { * Object with 'styelSheet' property for form on new actor. */ addStyleSheet: function (text) { + // This is a bit convoluted. The style sheet actor may be created + // by a notification from platform. In this case, we can't easily + // pass the "new" flag through to createStyleSheetActor, so we set + // a flag locally and check it before sending an event to the + // client. See |_onNewStyleSheetActor|. + this._nextStyleSheetIsNew = true; + let parent = this.document.documentElement; let style = this.document.createElementNS("http://www.w3.org/1999/xhtml", "style"); style.setAttribute("type", "text/css"); diff --git a/devtools/shared/specs/stylesheets.js b/devtools/shared/specs/stylesheets.js index c89a7c088..fc75281f8 100644 --- a/devtools/shared/specs/stylesheets.js +++ b/devtools/shared/specs/stylesheets.js @@ -105,6 +105,14 @@ exports.styleSheetSpec = styleSheetSpec; const styleSheetsSpec = generateActorSpec({ typeName: "stylesheets", + events: { + "stylesheet-added": { + type: "stylesheetAdded", + sheet: Arg(0, "stylesheet"), + isNew: Arg(1, "boolean") + }, + }, + methods: { getStyleSheets: { request: {}, -- cgit v1.2.3