Bug 1458749 - Remove checks for old traits in the inspector. r=pbro
authorGabriel Luong <gabriel.luong@gmail.com>
Fri, 04 May 2018 17:37:28 -0400
changeset 473143 f64dc995864603030fd080330b1991c02ff61cad
parent 473142 9577b0cdcad5114267bf422cf5f2e515cde5c27a
child 473144 3b55fcc68a6d1b61d9ad38304a4df86115299392
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1458749
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1458749 - Remove checks for old traits in the inspector. r=pbro
devtools/client/framework/test/browser_target_support.js
devtools/client/framework/toolbox-highlighter-utils.js
devtools/client/inspector/inspector.js
devtools/client/inspector/rules/rules.js
devtools/client/inspector/shared/highlighters-overlay.js
devtools/client/inspector/shared/tooltips-overlay.js
devtools/client/styleeditor/StyleEditorUI.jsm
devtools/server/actors/root.js
--- a/devtools/client/framework/test/browser_target_support.js
+++ b/devtools/client/framework/test/browser_target_support.js
@@ -37,18 +37,16 @@ async function testTarget(client, target
     "target.actorHasMethod() returns false for existing actor with no method");
   hasMethod = await target.actorHasMethod("nope", "nope");
   is(hasMethod, false,
     "target.actorHasMethod() returns false for non-existing actor with no method");
   hasMethod = await target.actorHasMethod();
   is(hasMethod, false,
     "target.actorHasMethod() returns false for undefined params");
 
-  is(target.getTrait("customHighlighters"), true,
-    "target.getTrait() returns boolean when trait exists");
   is(target.getTrait("giddyup"), undefined,
     "target.getTrait() returns undefined when trait does not exist");
 
   close(target, client);
 }
 
 // Ensure target is closed if client is closed directly
 function test() {
--- a/devtools/client/framework/toolbox-highlighter-utils.js
+++ b/devtools/client/framework/toolbox-highlighter-utils.js
@@ -58,23 +58,16 @@ exports.getHighlighterUtils = function(t
    * which doesn't have the highlighter actor. This can be removed as soon as
    * the minimal supported version becomes 1.4 (29)
    */
   let isRemoteHighlightable = exported.isRemoteHighlightable = function() {
     return target.client.traits.highlightable;
   };
 
   /**
-   * Does the target support custom highlighters.
-   */
-  let supportsCustomHighlighters = exported.supportsCustomHighlighters = () => {
-    return !!target.client.traits.customHighlighters;
-  };
-
-  /**
    * Make a function that initializes the inspector before it runs.
    * Since the init of the inspector is asynchronous, the return value will be
    * produced by Task.async and the argument should be a generator
    * @param {Function*} generator A generator function
    * @return {Function} A function
    */
   let isInspectorInitialized = false;
   let requireInspector = generator => {
@@ -298,21 +291,17 @@ exports.getHighlighterUtils = function(t
    * highlighters are needed in parallel, this method can be used to return a
    * new instance of a highlighter actor, given a type.
    * The type of the highlighter passed must be known by the server.
    * The highlighter actor returned will have the show(nodeFront) and hide()
    * methods and needs to be released by the consumer when not needed anymore.
    * @return a promise that resolves to the highlighter
    */
   exported.getHighlighterByType = requireInspector(async function(typeName) {
-    let highlighter = null;
-
-    if (supportsCustomHighlighters()) {
-      highlighter = await toolbox.inspector.getHighlighterByType(typeName);
-    }
+    let highlighter = await toolbox.inspector.getHighlighterByType(typeName);
 
     return highlighter || promise.reject("The target doesn't support " +
         `creating highlighters by types or ${typeName} is unknown`);
   });
 
   // Return the public API
   return exported;
 };
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -183,44 +183,26 @@ Inspector.prototype = {
   get selection() {
     return this._toolbox.selection;
   },
 
   get highlighter() {
     return this._toolbox.highlighter;
   },
 
-  get isOuterHTMLEditable() {
-    return this._target.client.traits.editOuterHTML;
-  },
-
-  get hasUrlToImageDataResolver() {
-    return this._target.client.traits.urlToImageDataResolver;
-  },
-
-  get canGetUniqueSelector() {
-    return this._target.client.traits.getUniqueSelector;
-  },
-
+  // Added in 53.
   get canGetCssPath() {
     return this._target.client.traits.getCssPath;
   },
 
+  // Added in 56.
   get canGetXPath() {
     return this._target.client.traits.getXPath;
   },
 
-  get canGetUsedFontFaces() {
-    return this._target.client.traits.getUsedFontFaces;
-  },
-
-  get canPasteInnerOrAdjacentHTML() {
-    return this._target.client.traits.pasteHTML;
-  },
-
   /**
    * Handle promise rejections for various asynchronous actions, and only log errors if
    * the inspector panel still exists.
    * This is useful to silence useless errors that happen when the inspector is closed
    * while still initializing (and making protocol requests).
    */
   _handleRejectionIfNotDestroyed: function(e) {
     if (!this._panelDestroyer) {
@@ -836,73 +818,69 @@ Inspector.prototype = {
     // Rules, Layout, Computed, etc.
     if (this.show3PaneToggle) {
       this.sidebar.addExistingTab(
         "computedview",
         INSPECTOR_L10N.getStr("inspector.sidebar.computedViewTitle"),
         defaultTab == "computedview");
     }
 
-    if (this.target.form.animationsActor) {
-      const animationTitle =
-        INSPECTOR_L10N.getStr("inspector.sidebar.animationInspectorTitle");
+    const animationTitle =
+      INSPECTOR_L10N.getStr("inspector.sidebar.animationInspectorTitle");
 
-      if (Services.prefs.getBoolPref("devtools.new-animationinspector.enabled")) {
-        const animationId = "newanimationinspector";
+    if (Services.prefs.getBoolPref("devtools.new-animationinspector.enabled")) {
+      const animationId = "newanimationinspector";
 
-        this.sidebar.addTab(
-          animationId,
-          animationTitle,
-          {
-            props: {
-              id: animationId,
-              title: animationTitle
-            },
-            panel: () => {
-              const AnimationInspector =
-                this.browserRequire("devtools/client/inspector/animation/animation");
-              this.animationinspector = new AnimationInspector(this, this.panelWin);
-              return this.animationinspector.provider;
-            }
+      this.sidebar.addTab(
+        animationId,
+        animationTitle,
+        {
+          props: {
+            id: animationId,
+            title: animationTitle
           },
-          defaultTab == animationId);
-      } else {
-        this.sidebar.addFrameTab(
-          "animationinspector",
-          animationTitle,
-          "chrome://devtools/content/inspector/animation-old/animation-inspector.xhtml",
-          defaultTab == "animationinspector");
-      }
+          panel: () => {
+            const AnimationInspector =
+              this.browserRequire("devtools/client/inspector/animation/animation");
+            this.animationinspector = new AnimationInspector(this, this.panelWin);
+            return this.animationinspector.provider;
+          }
+        },
+        defaultTab == animationId);
+    } else {
+      this.sidebar.addFrameTab(
+        "animationinspector",
+        animationTitle,
+        "chrome://devtools/content/inspector/animation-old/animation-inspector.xhtml",
+        defaultTab == "animationinspector");
     }
 
-    if (this.canGetUsedFontFaces) {
-      // Inject a lazy loaded react tab by exposing a fake React object
-      // with a lazy defined Tab thanks to `panel` being a function
-      let fontId = "fontinspector";
-      let fontTitle = INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle");
-      this.sidebar.addTab(
-        fontId,
-        fontTitle,
-        {
-          props: {
-            id: fontId,
-            title: fontTitle
-          },
-          panel: () => {
-            if (!this.fontinspector) {
-              const FontInspector =
-                this.browserRequire("devtools/client/inspector/fonts/fonts");
-              this.fontinspector = new FontInspector(this, this.panelWin);
-            }
+    // Inject a lazy loaded react tab by exposing a fake React object
+    // with a lazy defined Tab thanks to `panel` being a function
+    let fontId = "fontinspector";
+    let fontTitle = INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle");
+    this.sidebar.addTab(
+      fontId,
+      fontTitle,
+      {
+        props: {
+          id: fontId,
+          title: fontTitle
+        },
+        panel: () => {
+          if (!this.fontinspector) {
+            const FontInspector =
+              this.browserRequire("devtools/client/inspector/fonts/fonts");
+            this.fontinspector = new FontInspector(this, this.panelWin);
+          }
 
-            return this.fontinspector.provider;
-          }
-        },
-        defaultTab == fontId);
-    }
+          return this.fontinspector.provider;
+        }
+      },
+      defaultTab == fontId);
 
     // Persist splitter state in preferences.
     this.sidebar.on("show", this.onSidebarShown);
     this.sidebar.on("hide", this.onSidebarHidden);
     this.sidebar.on("destroy", this.onSidebarHidden);
 
     this.sidebar.show(defaultTab);
   },
@@ -1196,18 +1174,17 @@ Inspector.prototype = {
     if (this.canAddHTMLChild()) {
       btn.removeAttribute("disabled");
     } else {
       btn.setAttribute("disabled", "true");
     }
 
     // On any new selection made by the user, store the unique css selector
     // of the selected node so it can be restored after reload of the same page
-    if (this.canGetUniqueSelector &&
-        this.selection.isElementNode()) {
+    if (this.selection.isElementNode()) {
       selection.getUniqueSelector().then(selector => {
         this.selectionCssSelector = selector;
       }, this._handleRejectionIfNotDestroyed);
     }
 
     let selfUpdate = this.updating("inspector-panel");
     executeSoon(() => {
       try {
@@ -1415,25 +1392,24 @@ Inspector.prototype = {
     let isSelectionElement = this.selection.isElementNode() &&
                              !this.selection.isPseudoElementNode();
     let isEditableElement = isSelectionElement &&
                             !this.selection.isAnonymousNode();
     let isDuplicatableElement = isSelectionElement &&
                                 !this.selection.isAnonymousNode() &&
                                 !this.selection.isRoot();
     let isScreenshotable = isSelectionElement &&
-                           this.canGetUniqueSelector &&
                            this.selection.nodeFront.isTreeDisplayed;
 
     let menu = new Menu();
     menu.append(new MenuItem({
       id: "node-menu-edithtml",
       label: INSPECTOR_L10N.getStr("inspectorHTMLEdit.label"),
       accesskey: INSPECTOR_L10N.getStr("inspectorHTMLEdit.accesskey"),
-      disabled: !isEditableElement || !this.isOuterHTMLEditable,
+      disabled: !isEditableElement,
       click: () => this.editHTML(),
     }));
     menu.append(new MenuItem({
       id: "node-menu-add",
       label: INSPECTOR_L10N.getStr("inspectorAddNode.label"),
       accesskey: INSPECTOR_L10N.getStr("inspectorAddNode.accesskey"),
       disabled: !this.canAddHTMLChild(),
       click: () => this.addNode(),
@@ -1611,17 +1587,16 @@ Inspector.prototype = {
       click: () => this.copyOuterHTML(),
     }));
     copySubmenu.append(new MenuItem({
       id: "node-menu-copyuniqueselector",
       label: INSPECTOR_L10N.getStr("inspectorCopyCSSSelector.label"),
       accesskey:
         INSPECTOR_L10N.getStr("inspectorCopyCSSSelector.accesskey"),
       disabled: !isSelectionElement,
-      hidden: !this.canGetUniqueSelector,
       click: () => this.copyUniqueSelector(),
     }));
     copySubmenu.append(new MenuItem({
       id: "node-menu-copycsspath",
       label: INSPECTOR_L10N.getStr("inspectorCopyCSSPath.label"),
       accesskey:
         INSPECTOR_L10N.getStr("inspectorCopyCSSPath.accesskey"),
       disabled: !isSelectionElement,
@@ -1645,36 +1620,34 @@ Inspector.prototype = {
       click: () => this.copyImageDataUri(),
     }));
 
     return copySubmenu;
   },
 
   _getPasteSubmenu: function(isEditableElement) {
     let isPasteable = isEditableElement && this._getClipboardContentForPaste();
-    let disableAdjacentPaste = !isPasteable ||
-          !this.canPasteInnerOrAdjacentHTML || this.selection.isRoot() ||
+    let disableAdjacentPaste = !isPasteable || this.selection.isRoot() ||
           this.selection.isBodyNode() || this.selection.isHeadNode();
     let disableFirstLastPaste = !isPasteable ||
-          !this.canPasteInnerOrAdjacentHTML || (this.selection.isHTMLNode() &&
-          this.selection.isRoot());
+          (this.selection.isHTMLNode() && this.selection.isRoot());
 
     let pasteSubmenu = new Menu();
     pasteSubmenu.append(new MenuItem({
       id: "node-menu-pasteinnerhtml",
       label: INSPECTOR_L10N.getStr("inspectorPasteInnerHTML.label"),
       accesskey: INSPECTOR_L10N.getStr("inspectorPasteInnerHTML.accesskey"),
-      disabled: !isPasteable || !this.canPasteInnerOrAdjacentHTML,
+      disabled: !isPasteable,
       click: () => this.pasteInnerHTML(),
     }));
     pasteSubmenu.append(new MenuItem({
       id: "node-menu-pasteouterhtml",
       label: INSPECTOR_L10N.getStr("inspectorPasteOuterHTML.label"),
       accesskey: INSPECTOR_L10N.getStr("inspectorPasteOuterHTML.accesskey"),
-      disabled: !isPasteable || !this.isOuterHTMLEditable,
+      disabled: !isPasteable,
       click: () => this.pasteOuterHTML(),
     }));
     pasteSubmenu.append(new MenuItem({
       id: "node-menu-pastebefore",
       label: INSPECTOR_L10N.getStr("inspectorHTMLPasteBefore.label"),
       accesskey:
         INSPECTOR_L10N.getStr("inspectorHTMLPasteBefore.accesskey"),
       disabled: disableAdjacentPaste,
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -234,26 +234,22 @@ CssRuleView.prototype = {
    *
    * @return {Promise} Resolves to the instance of the highlighter.
    */
   async getSelectorHighlighter() {
     if (!this.inspector) {
       return null;
     }
 
-    let utils = this.inspector.toolbox.highlighterUtils;
-    if (!utils.supportsCustomHighlighters()) {
-      return null;
-    }
-
     if (this.selectorHighlighter) {
       return this.selectorHighlighter;
     }
 
     try {
+      let utils = this.inspector.toolbox.highlighterUtils;
       let h = await utils.getHighlighterByType("SelectorHighlighter");
       this.selectorHighlighter = h;
       return h;
     } catch (e) {
       // The SelectorHighlighter type could not be created in the
       // current target.  It could be an older server, or a XUL page.
       return null;
     }
@@ -540,23 +536,18 @@ CssRuleView.prototype = {
   },
 
   /**
    * Add a new rule to the current element.
    */
   _onAddRule: function() {
     let elementStyle = this._elementStyle;
     let element = elementStyle.element;
-    let client = this.inspector.target.client;
     let pseudoClasses = element.pseudoClassLocks;
 
-    if (!client.traits.addNewRule) {
-      return;
-    }
-
     if (!this.pageStyle.supportsAuthoredStyles) {
       // We're talking to an old server.
       this._onAddNewRuleNonAuthored();
       return;
     }
 
     // Adding a new rule with authored styles will cause the actor to
     // emit an event, which will in turn cause the rule view to be
--- a/devtools/client/inspector/shared/highlighters-overlay.js
+++ b/devtools/client/inspector/shared/highlighters-overlay.js
@@ -43,19 +43,16 @@ class HighlightersOverlay {
     * behave like highlighters but with added editing capabilities that need to map value
     * changes to properties in the Rule view.
     */
     this.editors = {};
     this.inspector = inspector;
     this.highlighterUtils = this.inspector.toolbox.highlighterUtils;
     this.store = this.inspector.store;
 
-    // Only initialize the overlay if at least one of the highlighter types is supported.
-    this.supportsHighlighters = this.highlighterUtils.supportsCustomHighlighters();
-
     // NodeFront of the flexbox container that is highlighted.
     this.flexboxHighlighterShown = null;
     // NodeFront of element that is highlighted by the geometry editor.
     this.geometryEditorHighlighterShown = null;
     // NodeFront of the grid container that is highlighted.
     this.gridHighlighterShown = null;
     // Name of the highlighter shown on mouse hover.
     this.hoveredHighlighterShown = null;
@@ -107,40 +104,32 @@ class HighlightersOverlay {
   /**
    * Add the highlighters overlay to the view. This will start tracking mouse events
    * and display highlighters when needed.
    *
    * @param  {CssRuleView|CssComputedView|LayoutView} view
    *         Either the rule-view or computed-view panel to add the highlighters overlay.
    */
   addToView(view) {
-    if (!this.supportsHighlighters) {
-      return;
-    }
-
     let el = view.element;
     el.addEventListener("click", this.onClick, true);
     el.addEventListener("mousemove", this.onMouseMove);
     el.addEventListener("mouseout", this.onMouseOut);
     el.ownerDocument.defaultView.addEventListener("mouseout", this.onMouseOut);
   }
 
   /**
    * Remove the overlay from the given view. This will stop tracking mouse movement and
    * showing highlighters.
    *
    * @param  {CssRuleView|CssComputedView|LayoutView} view
    *         Either the rule-view or computed-view panel to remove the highlighters
    *         overlay.
    */
   removeFromView(view) {
-    if (!this.supportsHighlighters) {
-      return;
-    }
-
     let el = view.element;
     el.removeEventListener("click", this.onClick, true);
     el.removeEventListener("mousemove", this.onMouseMove);
     el.removeEventListener("mouseout", this.onMouseOut);
   }
 
   /**
    * Load the grid highligher display settings into the store from the stored preferences.
@@ -989,17 +978,16 @@ class HighlightersOverlay {
     // Remove inspector events.
     this.inspector.off("markupmutation", this.onMarkupMutation);
     this.inspector.target.off("will-navigate", this.onWillNavigate);
 
     this._lastHovered = null;
 
     this.inspector = null;
     this.highlighterUtils = null;
-    this.supportsHighlighters = null;
     this.state = null;
     this.store = null;
 
     this.boxModelHighlighterShown = null;
     this.flexboxHighlighterShown = null;
     this.geometryEditorHighlighterShown = null;
     this.gridHighlighterShown = null;
     this.hoveredHighlighterShown = null;
--- a/devtools/client/inspector/shared/tooltips-overlay.js
+++ b/devtools/client/inspector/shared/tooltips-overlay.js
@@ -157,21 +157,19 @@ TooltipsOverlay.prototype = {
    * Given a hovered node info, find out which type of tooltip should be shown,
    * if any
    *
    * @param {Object} nodeInfo
    * @return {String} The tooltip type to be shown, or null
    */
   _getTooltipType: function({type, value: prop}) {
     let tooltipType = null;
-    let inspector = this.view.inspector;
 
     // Image preview tooltip
-    if (type === VIEW_NODE_IMAGE_URL_TYPE &&
-        inspector.hasUrlToImageDataResolver) {
+    if (type === VIEW_NODE_IMAGE_URL_TYPE) {
       tooltipType = TOOLTIP_IMAGE_TYPE;
     }
 
     // Font preview tooltip
     if ((type === VIEW_NODE_VALUE_TYPE && prop.property === "font-family") ||
         (type === VIEW_NODE_FONT_TYPE)) {
       let value = prop.value.toLowerCase();
       if (value !== "inherit" && value !== "unset" && value !== "initial") {
--- a/devtools/client/styleeditor/StyleEditorUI.jsm
+++ b/devtools/client/styleeditor/StyleEditorUI.jsm
@@ -123,27 +123,26 @@ StyleEditorUI.prototype = {
   },
 
   async initializeHighlighter() {
     let toolbox = gDevTools.getToolbox(this._target);
     await toolbox.initInspector();
     this._walker = toolbox.walker;
 
     let hUtils = toolbox.highlighterUtils;
-    if (hUtils.supportsCustomHighlighters()) {
-      try {
-        this._highlighter =
-          await hUtils.getHighlighterByType(SELECTOR_HIGHLIGHTER_TYPE);
-      } catch (e) {
-        // The selectorHighlighter can't always be instantiated, for example
-        // it doesn't work with XUL windows (until bug 1094959 gets fixed);
-        // or the selectorHighlighter doesn't exist on the backend.
-        console.warn("The selectorHighlighter couldn't be instantiated, " +
-          "elements matching hovered selectors will not be highlighted");
-      }
+
+    try {
+      this._highlighter =
+        await hUtils.getHighlighterByType(SELECTOR_HIGHLIGHTER_TYPE);
+    } catch (e) {
+      // The selectorHighlighter can't always be instantiated, for example
+      // it doesn't work with XUL windows (until bug 1094959 gets fixed);
+      // or the selectorHighlighter doesn't exist on the backend.
+      console.warn("The selectorHighlighter couldn't be instantiated, " +
+        "elements matching hovered selectors will not be highlighted");
     }
   },
 
   /**
    * Build the initial UI and wire buttons with event handlers.
    */
   createUI: function() {
     let viewRoot = this._root.parentNode.querySelector(".splitview-root");
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -107,65 +107,45 @@ function RootActor(connection, parameter
 }
 
 RootActor.prototype = {
   constructor: RootActor,
   applicationType: "browser",
 
   traits: {
     sources: true,
-    // Whether the inspector actor allows modifying outer HTML.
-    editOuterHTML: true,
-    // Whether the inspector actor allows modifying innerHTML and inserting
-    // adjacent HTML.
-    pasteHTML: true,
     // Whether the server-side highlighter actor exists and can be used to
     // remotely highlight nodes (see server/actors/highlighters.js)
     highlightable: true,
-    // Which custom highlighter does the server-side highlighter actor supports?
-    // (see server/actors/highlighters.js)
-    customHighlighters: true,
-    // Whether the inspector actor implements the getImageDataFromURL
-    // method that returns data-uris for image URLs. This is used for image
-    // tooltips for instance
-    urlToImageDataResolver: true,
     networkMonitor: true,
     // Whether the storage inspector actor to inspect cookies, etc.
     storageInspector: true,
     // Whether storage inspector is read only
     storageInspectorReadOnly: true,
     // Whether conditional breakpoints are supported
     conditionalBreakpoints: true,
     // Whether the server supports full source actors (breakpoints on
     // eval scripts, etc)
     debuggerSourceActors: true,
     // Whether the server can return wasm binary source
     wasmBinarySource: true,
     bulk: true,
     // Whether the style rule actor implements the modifySelector method
     // that modifies the rule's selector
     selectorEditable: true,
-    // Whether the page style actor implements the addNewRule method that
-    // adds new rules to the page
-    addNewRule: true,
-    // Whether the dom node actor implements the getUniqueSelector method
-    getUniqueSelector: true,
-    // Whether the dom node actor implements the getCssPath method
+    // Whether the dom node actor implements the getCssPath method. Added in 53.
     getCssPath: true,
-    // Whether the dom node actor implements the getXPath method
+    // Whether the dom node actor implements the getXPath method. Added in 56.
     getXPath: true,
     // Whether the director scripts are supported
     directorScripts: true,
     // Whether the debugger server supports
     // blackboxing/pretty-printing (not supported in Fever Dream yet)
     noBlackBoxing: false,
     noPrettyPrinting: false,
-    // Whether the page style actor implements the getUsedFontFaces method
-    // that returns the font faces used on a node
-    getUsedFontFaces: true,
     // Trait added in Gecko 38, indicating that all features necessary for
     // grabbing allocations from the MemoryActor are available for the performance tool
     memoryActorAllocations: true,
     // Added in Firefox 40. Indicates that the backend supports registering custom
     // commands through the WebConsoleCommands API.
     webConsoleCommands: true,
     // Whether root actor exposes tab actors and access to any window.
     // If allowChromeProcess is true, you can: