Bug 1013887 - Make CSS coverage should look nicer; r=harth

2014-05-26 19:57:58 +01:00 · 2014-05-26 19:57:58 +01:00 · cbc6b76c9a
--- a/browser/devtools/styleeditor/styleeditor.css
+++ b/browser/devtools/styleeditor/styleeditor.css
@ -7,19 +7,6 @@
  -moz-box-flex: 1;
 }

-.csscoverage-report-container {
-  -moz-box-flex: 1;
-}
-
-.csscoverage-report {
-  -moz-box-orient: horizontal;
-}
-
-.csscoverage-report-container {
-  overflow-x: hidden;
-  overflow-y: auto;
-}
-
 .stylesheet-error-message {
  display: none;
 }
@ -142,3 +129,21 @@ li:hover > hgroup > .stylesheet-more > h3 > .stylesheet-saveButton {
    -moz-box-flex: 0;
  }
 }
+
+.csscoverage-report-container {
+  -moz-box-flex: 1;
+  overflow-x: hidden;
+  overflow-y: auto;
+}
+
+.csscoverage-report-content > * {
+  display: inline-block;
+}
+
+.csscoverage-report {
+  -moz-box-orient: horizontal;
+}
+
+.csscoverage-report .pie-table-chart-container {
+  -moz-box-orient: vertical;
+}
--- a/browser/devtools/styleeditor/styleeditor.xul
+++ b/browser/devtools/styleeditor/styleeditor.xul
@ -14,8 +14,10 @@
 ]>

 <?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
+<?xml-stylesheet href="chrome://browser/content/devtools/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://browser/content/devtools/splitview.css" type="text/css"?>
 <?xml-stylesheet href="chrome://browser/skin/devtools/common.css" type="text/css"?>
+<?xml-stylesheet href="chrome://browser/skin/devtools/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://browser/skin/devtools/splitview.css" type="text/css"?>
 <?xml-stylesheet href="chrome://browser/content/devtools/styleeditor.css" type="text/css"?>
 <?xml-stylesheet href="chrome://browser/skin/devtools/styleeditor.css" type="text/css"?>
@ -152,39 +154,43 @@
      <!-- The data for this comes from UsageReportActor.createPageReport -->
      <div class="csscoverage-report-container">
        <div class="csscoverage-report-content">
-          <h2>&csscoverage.unused;</h2>
-          <p>&csscoverage.noMatch;</p>
-          <ul>
-            <li foreach="rule in ${unusedRules}">
-              <code>${rule.selectorText}</code>
-              <span class="link"
-                  title="${rule.url}">(${rule.shortHref} : ${rule.start.line})</span>
-            </li>
-          </ul>
-          <h2>&csscoverage.optimize;</h2>
-          <p>
-            &csscoverage.preload1;
-            <code>&lt;link ...></code>
-            &csscoverage.preload2;
-            <code>&lt;style>...</code>
-            &csscoverage.preload3;
-          </p>
-          <div if="${pages.length == 0}">
-            &csscoverage.noPreload;
+          <div class="csscoverage-report-summary">
+            <div class="csscoverage-report-chart"/>
          </div>
-          <div if="${pages.length > 0}">
-            <div foreach="page in ${pages}">
+          <div class="csscoverage-report-unused">
+            <h2>&csscoverage.unused;</h2>
+            <p>&csscoverage.noMatch;</p>
+            <div foreach="page in ${unused}">
              <h3>${page.url}</h3>
-              <textarea>&lt;style>
-<loop foreach="rule in ${page.preloadRules}"
-                    onclick="${rule.onclick}">${rule.formattedCssText}</loop>&lt;/style></textarea>
+              <code foreach="rule in ${page.rules}"
+                    href="${rule.url}"
+                    class="csscoverage-list">${rule.selectorText}</code>
            </div>
          </div>
-          <p>
-            &csscoverage.footer1;
-            <a target="_blank" href="&csscoverage.footer2;">&csscoverage.footer3;</a>
-            &csscoverage.footer4;
-          </p>
+          <div class="csscoverage-report-optimize">
+            <h2>&csscoverage.optimize;</h2>
+            <p>
+              &csscoverage.preload1;
+              <code>&lt;link ...></code>
+              &csscoverage.preload2;
+              <code>&lt;style>...</code>
+              &csscoverage.preload3;
+            </p>
+            <div if="${preload.length == 0}">&csscoverage.noPreload;</div>
+            <div if="${preload.length > 0}">
+              <div foreach="page in ${preload}">
+                <h3>${page.url}</h3>
+                <textarea>&lt;style>
+<loop foreach="rule in ${page.rules}"
+                      onclick="${rule.onclick}">${rule.formattedCssText}</loop>&lt;/style></textarea>
+              </div>
+            </div>
+            <p>
+              &csscoverage.footer1;
+              <a target="_blank" href="&csscoverage.footer2;">&csscoverage.footer3;</a>
+              &csscoverage.footer4;
+            </p>
+          </div>
          <p>&#160;</p>
        </div>
      </div>
--- a/browser/themes/shared/devtools/dark-theme.css
+++ b/browser/themes/shared/devtools/dark-theme.css
@ -103,7 +103,7 @@

 .cm-s-mozilla .cm-unused-line {
  text-decoration: line-through;
-  -moz-text-decoration-color: red;
+  -moz-text-decoration-color: #5f88b0;
 }

 .cm-s-mozilla .cm-executed-line {
--- a/browser/themes/shared/devtools/light-theme.css
+++ b/browser/themes/shared/devtools/light-theme.css
@ -76,7 +76,7 @@

 .cm-s-mozilla .cm-unused-line {
  text-decoration: line-through;
-  -moz-text-decoration-color: red;
+  -moz-text-decoration-color: #5f88b0;
 }

 .cm-s-mozilla .cm-executed-line {
--- a/browser/themes/shared/devtools/styleeditor.css
+++ b/browser/themes/shared/devtools/styleeditor.css
@ -3,54 +3,6 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

-.theme-light .csscoverage-report {
-  background: url(background-noise-toolbar.png), #f0f1f2; /* Toolbars */
-}
-
-.theme-dark .csscoverage-report {
-  background: url(background-noise-toolbar.png), #343c45; /* Toolbars */
-}
-
-.csscoverage-report-container {
-  height: 100vh;
-  padding: 10px;
-}
-
-.csscoverage-report-content {
-  font-size: 13px;
-  margin: 0 auto;
-  max-width: 600px;
-  padding: 0 10px;
-}
-
-.csscoverage-report h1,
-.csscoverage-report h2,
-.csscoverage-report h3 {
-  font-weight: bold;
-}
-
-.csscoverage-report textarea {
-  width: 100%;
-  height: 100px;
-}
-
-.csscoverage-report > .csscoverage-toolbar {
-  border: none;
-  margin: 0;
-  padding: 0;
-}
-
-.csscoverage-report > .csscoverage-toolbarbutton {
-  min-width: 4em;
-  min-height: 100vh;
-  margin: 0;
-  padding: 0;
-  border-radius: 0;
-  border-top: none;
-  border-bottom: none;
-  -moz-border-start: none;
-}
-
 .stylesheet-title,
 .stylesheet-name {
  text-decoration: none;
@ -239,3 +191,121 @@ h3 {
    width: 180px;
  }
 }
+
+.theme-light .csscoverage-report {
+  background: url(background-noise-toolbar.png), #f0f1f2; /* Toolbars */
+}
+
+.theme-dark .csscoverage-report {
+  background: url(background-noise-toolbar.png), #343c45; /* Toolbars */
+}
+
+.csscoverage-report-container {
+  height: 100vh;
+  padding: 0 30px;
+}
+
+.csscoverage-report-content {
+  margin: 20px auto;
+  -moz-column-width: 300px;
+  font-size: 13px;
+}
+
+.csscoverage-report h1 {
+  font-size: 120%;
+}
+
+.csscoverage-report h2 {
+  font-size: 110%;
+}
+
+.csscoverage-report h1,
+.csscoverage-report h2,
+.csscoverage-report h3 {
+  font-weight: bold;
+  margin: 10px 0;
+}
+
+.csscoverage-list:after {
+  content: ', ';
+}
+
+.csscoverage-list:last-child:after {
+  display: none;
+}
+
+.csscoverage-report textarea {
+  width: 100%;
+  height: 100px;
+}
+
+.csscoverage-report a {
+  cursor: pointer;
+  text-decoration: underline;
+}
+
+.csscoverage-report > .csscoverage-toolbar {
+  border: none;
+  margin: 0;
+  padding: 0;
+}
+
+.csscoverage-report > .csscoverage-toolbarbutton {
+  min-width: 4em;
+  min-height: 100vh;
+  margin: 0;
+  padding: 0;
+  border-radius: 0;
+  border-top: none;
+  border-bottom: none;
+  -moz-border-start: none;
+}
+
+.theme-dark .chart-colored-blob[name="Used Preload"] {
+  fill: #df80ff; /* Pink highlight */
+  background: #df80ff;
+}
+
+.theme-light .chart-colored-blob[name="Used Preload"] {
+  fill: #b82ee5; /* Pink highlight */
+  background: #b82ee5;
+}
+
+.theme-dark .chart-colored-blob[name=Used] {
+  fill: #70bf53; /* Green highlight */
+  background: #70bf53;
+}
+
+.theme-light .chart-colored-blob[name=Used] {
+  fill: #2cbb0f; /* Green highlight */
+  background: #2cbb0f;
+}
+
+.theme-dark .chart-colored-blob[name=Unused] {
+  fill: #d99b28; /* Light Orange highlight */
+  background: #d99b28;
+}
+
+.theme-light .chart-colored-blob[name=Unused] {
+  fill: #d97e00; /* Light Orange highlight */
+  background: #d97e00;
+}
+
+/* Undo 'largest' customization */
+.theme-dark .pie-chart-slice[largest] {
+  stroke-width: 1px;
+  stroke: rgba(0,0,0,0.2);
+}
+
+.theme-light .pie-chart-slice[largest] {
+  stroke-width: 1px;
+  stroke: rgba(255,255,255,0.8);
+}
+
+.csscoverage-report .pie-chart-slice {
+  cursor: default;
+}
+
+.csscoverage-report-chart {
+  margin: 0 50px;
+}
--- a/toolkit/devtools/gcli/commands/csscoverage.js
+++ b/toolkit/devtools/gcli/commands/csscoverage.js
@ -13,6 +13,10 @@ const domtemplate = require("gcli/util/domtemplate");
 const csscoverage = require("devtools/server/actors/csscoverage");
 const l10n = csscoverage.l10n;

+const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "Chart", "resource:///modules/devtools/Chart.jsm");
+
 /**
 * The commands/converters for GCLI
 */
@ -101,8 +105,9 @@ exports.items = [
      templ.hidden = false;

      let data = {
-        pages: csscoveragePageReport.pages,
-        unusedRules: csscoveragePageReport.unusedRules,
+        preload: csscoveragePageReport.preload,
+        unused: csscoveragePageReport.unused,
+        summary: csscoveragePageReport.summary,
        onback: () => {
          // The back button clears and hides .csscoverage-report
          while (host.hasChildNodes()) {
@ -118,11 +123,12 @@ exports.items = [
        };
      };

-      data.pages.forEach(page => {
-        page.preloadRules.forEach(addOnClick);
+      data.preload.forEach(page => {
+        page.rules.forEach(addOnClick);
+      });
+      data.unused.forEach(page => {
+        page.rules.forEach(addOnClick);
      });
-
-      data.unusedRules.forEach(addOnClick);

      let options = { allowEval: true, stack: "styleeditor.xul" };
      domtemplate.template(templ, data, options);
@ -130,6 +136,20 @@ exports.items = [
      while (templ.hasChildNodes()) {
        host.appendChild(templ.firstChild);
      }
+
+      // Create a new chart.
+      let container = host.querySelector(".csscoverage-report-chart");
+      let chart = Chart.PieTable(panel._panelDoc, {
+        diameter: 200, // px
+        title: "CSS Usage",
+        data: [
+          { size: data.summary.preload, label: "Used Preload" },
+          { size: data.summary.used, label: "Used" },
+          { size: data.summary.unused, label: "Unused" }
+        ]
+      });
+      container.appendChild(chart.node);
+
      host.hidden = false;
    }
  }
--- a/toolkit/devtools/server/actors/csscoverage.js
+++ b/toolkit/devtools/server/actors/csscoverage.js
@ -319,24 +319,34 @@ let UsageReportActor = protocol.ActorClass({
  /**
   * Returns a JSONable structure designed for the page report which shows
   * the recommended changes to a page.
+   *
+   * "preload" means that a rule is used before the load event happens, which
+   * means that the page could by optimized by placing it in a <style> element
+   * at the top of the page, moving the <link> elements to the bottom.
+   *
   * Example:
   *   {
-   *     pages: [
-   *      {
-   *        url: http://example.org/page1.html,
-   *        preloadRules: [
-   *          {
-   *            url: "http://example.org/style1.css",
-   *            start: { line: 3, column: 4 },
-   *            selectorText: "p#content",
-   *            formattedCssText: "p#content {\n  color: red;\n }\n",
-   *            onclick: function() { // open in style editor }
+   *     preload: [
+   *       {
+   *         url: "http://example.org/page1.html",
+   *         shortUrl: "page1.html",
+   *         rules: [
+   *           {
+   *             url: "http://example.org/style1.css",
+   *             shortUrl: "style1.css",
+   *             start: { line: 3, column: 4 },
+   *             selectorText: "p#content",
+   *             formattedCssText: "p#content {\n  color: red;\n }\n"
   *          },
   *          ...
-   *        ],
-   *        unusedRules: [
-   *          ...
-   *        ]
+   *         ]
+   *       }
+   *     ],
+   *     unused: [
+   *       {
+   *         url: "http://example.org/style1.css",
+   *         shortUrl: "style1.css",
+   *         rules: [ ... ]
   *       }
   *     ]
   *   }
@ -350,52 +360,76 @@ let UsageReportActor = protocol.ActorClass({
      throw new Error(l10n.lookup("csscoverageNotRunError"));
    }

-    // Create a JSONable data structure representing a rule
-    const ruleToRuleReport = function(ruleId, ruleData) {
-      let { url, line, column } = deconstructRuleId(ruleId);
+    // Helper function to create a JSONable data structure representing a rule
+    const ruleToRuleReport = function(rule, ruleData) {
      return {
-        url: url,
-        shortHref: url.split("/").slice(-1),
-        start: { line: line, column: column },
+        url: rule.url,
+        shortUrl: rule.url.split("/").slice(-1),
+        start: { line: rule.line, column: rule.column },
        selectorText: ruleData.selectorText,
        formattedCssText: prettifyCSS(ruleData.cssText)
      };
    }

-    let pages = [];
-    let unusedRules = [];
+    // A count of each type of rule for the bar chart
+    let summary = { used: 0, unused: 0, preload: 0 };

-    // Create a set of the unused rules
+    // Create the set of the unused rules
+    let unusedMap = new Map();
    for (let [ruleId, ruleData] of this._knownRules) {
-      if (!ruleData.isUsed) {
-        let ruleReport = ruleToRuleReport(ruleId, ruleData);
-        unusedRules.push(ruleReport);
+      let rule = deconstructRuleId(ruleId);
+      let rules = unusedMap.get(rule.url)
+      if (rules == null) {
+        rules = [];
+        unusedMap.set(rule.url, rules);
      }
+      if (!ruleData.isUsed) {
+        let ruleReport = ruleToRuleReport(rule, ruleData);
+        rules.push(ruleReport);
+      }
+      else {
+        summary.unused++;
+      }
+    }
+    let unused = [];
+    for (let [url, rules] of unusedMap) {
+      unused.push({
+        url: url,
+        shortUrl: url.split("/").slice(-1),
+        rules: rules
+      });
    }

    // Create the set of rules that could be pre-loaded
+    let preload = [];
    for (let url of this._visitedPages) {
      let page = {
        url: url,
-        shortHref: url.split("/").slice(-1),
-        preloadRules: []
+        shortUrl: url.split("/").slice(-1),
+        rules: []
      };

      for (let [ruleId, ruleData] of this._knownRules) {
        if (ruleData.preLoadOn.has(url)) {
-          let ruleReport = ruleToRuleReport(ruleId, ruleData);
-          page.preloadRules.push(ruleReport);
+          let rule = deconstructRuleId(ruleId);
+          let ruleReport = ruleToRuleReport(rule, ruleData);
+          page.rules.push(ruleReport);
+          summary.preload++;
+        }
+        else {
+          summary.used++;
        }
      }

-      if (page.preloadRules.length > 0) {
-        pages.push(page);
+      if (page.rules.length > 0) {
+        preload.push(page);
      }
    }

    return {
-      pages: pages,
-      unusedRules: unusedRules
+      summary: summary,
+      preload: preload,
+      unused: unused
    };
  }, {
    response: RetVal("json")
@ -658,12 +692,15 @@ const UsageReportFront = protocol.FrontClass(UsageReportActor, {
    this.manage(this);
  },

+  /**
+   * Server-side start is above. Client-side start adds a notification box
+   */
  start: custom(function(chromeWindow, target) {
    if (chromeWindow != null) {
      let gnb = chromeWindow.document.getElementById("global-notificationbox");
-      let notification = gnb.getNotificationWithValue("csscoverage-running");
+      this.notification = gnb.getNotificationWithValue("csscoverage-running");

-      if (!notification) {
+      if (this.notification == null) {
        let notifyStop = ev => {
          if (ev == "removed") {
            this.stop();
@ -671,19 +708,34 @@ const UsageReportFront = protocol.FrontClass(UsageReportActor, {
          }
        };

-        gnb.appendNotification(l10n.lookup("csscoverageRunningReply"),
-                               "csscoverage-running",
-                               "", // i.e. no image
-                               gnb.PRIORITY_INFO_HIGH,
-                               null, // i.e. no buttons
-                               notifyStop);
+        let msg = l10n.lookup("csscoverageRunningReply");
+        this.notification = gnb.appendNotification(msg,
+                                                   "csscoverage-running",
+                                                   "", // i.e. no image
+                                                   gnb.PRIORITY_INFO_HIGH,
+                                                   null, // i.e. no buttons
+                                                   notifyStop);
      }
    }

    return this._start();
  }, {
    impl: "_start"
-  })
+  }),
+
+  /**
+   * Client-side stop also removes the notification box
+   */
+  stop: custom(function() {
+    if (this.notification != null) {
+      this.notification.remove();
+      this.notification = undefined;
+    }
+
+    return this._stop();
+  }, {
+    impl: "_stop"
+  }),
 });

 exports.UsageReportFront = UsageReportFront;
--- a/toolkit/locales/en-US/chrome/global/devtools/csscoverage.dtd
+++ b/toolkit/locales/en-US/chrome/global/devtools/csscoverage.dtd
@ -19,7 +19,7 @@
 <!-- LOCALIZATION NOTE (csscoverage.unused, csscoverage.noMatch):
  -  This is the heading and body text for the CSS usage part of the report -->
 <!ENTITY csscoverage.unused "Unused Rules">
-<!ENTITY csscoverage.noMatch "The following selectors did not match any elements that we saw during the test so it's possible they can be safely removed.">
+<!ENTITY csscoverage.noMatch "No matches found for the following rules:">

 <!-- LOCALIZATION NOTE (csscoverage.optimize):
  -  This is the heading for the CSS optimization part of the report -->
@ -35,7 +35,7 @@

 <!-- LOCALIZATION NOTE (csscoverage.noPreload):
  -  This is what we say when we have no optimization suggestions -->
-<!ENTITY csscoverage.noPreload "All rules are inlined. We don't have suggestions about how to improve performance on this page right now.">
+<!ENTITY csscoverage.noPreload "All rules are inlined.">

 <!-- LOCALIZATION NOTE (csscoverage.footer1, csscoverage.footer2, csscoverage.footer3):
  -  The text displayed at the bottom of the page, with 2 being the URL opened