Bug 1582786 - Append the devtools theme stylesheet sheet after global.css in document.head if it exists instead of using an XML ProcessingInstruction r=nchevobbe
authorBrian Grinstead <bgrinstead@mozilla.com>
Wed, 25 Sep 2019 18:16:28 +0000
changeset 494959 8692e6c7b5e8795481f58863171d53c6b5026c2d
parent 494958 0cabfa57360c86cfa855ae918faccc2a3d51dca0
child 494960 eaa523b98e726c437fe96086f665495b61ed48e2
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1582786, 1581914
milestone71.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 1582786 - Append the devtools theme stylesheet sheet after global.css in document.head if it exists instead of using an XML ProcessingInstruction r=nchevobbe Otherwise, what happens in documents like the webconsole is the theme file gets loaded before global.css, which isn't the intended behavior and makes overriding the styles from global.css more difficult. As an example, some buttons in the webconsole became stretched after Bug 1581914 changed some default styling in global.css. This patch restores the correct behavior by loading the theme afer global.css. global.css is currently loaded in devtools in docs that explicitly use XUL elements (such as storage inspector and style editor), and in docs that need to be supported as top level windows (webconsole, toolbox, and browser toolbox). Unless if we change how things like panels and context menus are styled, the latter group will continue to need to load global.css even as XUL/XBL usage goes away (since they are styled with global.css). Differential Revision: https://phabricator.services.mozilla.com/D46530
devtools/client/framework/test/browser_toolbox_sidebar_tool.xul
devtools/client/framework/toolbox-process-window.html
devtools/client/framework/toolbox-window.xul
devtools/client/framework/toolbox.xul
devtools/client/performance/index.xul
devtools/client/shared/stylesheet-utils.js
devtools/client/shared/test/doc_options-view.xul
devtools/client/storage/index.xul
devtools/client/webconsole/index.html
--- a/devtools/client/framework/test/browser_toolbox_sidebar_tool.xul
+++ b/devtools/client/framework/test/browser_toolbox_sidebar_tool.xul
@@ -1,13 +1,13 @@
 <?xml version="1.0"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/content/shared/widgets/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/widgets.css" type="text/css"?>
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <script type="application/javascript" src="chrome://devtools/content/shared/theme-switching.js"/>
   <box flex="1" class="devtools-responsive-container theme-body">
     <vbox flex="1" class="devtools-main-content" id="content">test</vbox>
     <splitter class="devtools-side-splitter"/>
     <tabbox flex="1" id="sidebar" class="devtools-sidebar-tabs">
--- a/devtools/client/framework/toolbox-process-window.html
+++ b/devtools/client/framework/toolbox-process-window.html
@@ -2,17 +2,17 @@
    - 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/. -->
 <!DOCTYPE html>
 <html id="devtools-toolbox-window"
       windowtype="devtools:toolbox"
       width="900" height="350"
       persist="screenX screenY width height sizemode">
   <head>
-    <link rel="stylesheet" href="chrome://global/skin/"/>
+    <link rel="stylesheet" href="chrome://global/skin/global.css"/>
     <link rel="stylesheet" href="resource://devtools/client/themes/common.css"/>
     <link rel="stylesheet" href="chrome://devtools/content/framework/toolbox-process-window.css"/>
     <script src="chrome://global/content/globalOverlay.js"></script>
     <script src="toolbox-process-window.js"></script>
     <script src="chrome://global/content/viewSourceUtils.js"></script>
     <script src="chrome://browser/content/utilityOverlay.js"></script>
   </head>
   <body>
--- a/devtools/client/framework/toolbox-window.xul
+++ b/devtools/client/framework/toolbox-window.xul
@@ -1,14 +1,14 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
 
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 
 <!-- minwidth=50 is sum width of chevron and meatball menu button. -->
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         id="devtools-toolbox-window"
         macanimationtype="document"
         fullscreenbutton="true"
         windowtype="devtools:toolbox"
         width="900" height="320"
--- a/devtools/client/framework/toolbox.xul
+++ b/devtools/client/framework/toolbox.xul
@@ -1,13 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/toolbox.css" type="text/css"?>
 <?xml-stylesheet href="resource://devtools/client/shared/components/NotificationBox.css" type="text/css"?>
 <?xml-stylesheet href="resource://devtools/client/framework/components/DebugTargetErrorPage.css" type="text/css"?>
 
 <!DOCTYPE window [
 <!ENTITY % toolboxDTD SYSTEM "chrome://devtools/locale/toolbox.dtd" >
 %toolboxDTD;
 <!ENTITY % globalKeysDTD SYSTEM "chrome://global/locale/globalKeys.dtd">
--- a/devtools/client/performance/index.xul
+++ b/devtools/client/performance/index.xul
@@ -1,13 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/content/shared/widgets/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/performance.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/jit-optimizations.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/components-frame.css" type="text/css"?>
 <!DOCTYPE window [
   <!ENTITY % performanceDTD SYSTEM "chrome://devtools/locale/performance.dtd">
   %performanceDTD;
--- a/devtools/client/shared/stylesheet-utils.js
+++ b/devtools/client/shared/stylesheet-utils.js
@@ -1,45 +1,60 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 
 /* eslint-env browser */
 "use strict";
 
+function stylesheetLoadPromise(styleSheet, url) {
+  return new Promise((resolve, reject) => {
+    styleSheet.addEventListener("load", resolve, { once: true });
+    styleSheet.addEventListener("error", reject, { once: true });
+  });
+}
+
 /*
- * Append a stylesheet to the provided XUL document.
+ * Put the DevTools theme stylesheet into the provided chrome document.
  *
- * @param  {Document} xulDocument
- *         The XUL document where the stylesheet should be appended.
+ * @param  {Document} doc
+ *         The chrome document where the stylesheet should be appended.
  * @param  {String} url
  *         The url of the stylesheet to load.
  * @return {Object}
  *         - styleSheet {XMLStylesheetProcessingInstruction} the instruction node created.
  *         - loadPromise {Promise} that will resolve/reject when the stylesheet finishes
  *           or fails to load.
  */
-function appendStyleSheet(xulDocument, url) {
-  const styleSheetAttr = `href="${url}" type="text/css"`;
-  const styleSheet = xulDocument.createProcessingInstruction(
+function appendStyleSheet(doc, url) {
+  if (doc.head) {
+    const styleSheet = doc.createElement("link");
+    styleSheet.setAttribute("rel", "stylesheet");
+    styleSheet.setAttribute("href", url);
+    const loadPromise = stylesheetLoadPromise(styleSheet);
+
+    // In order to make overriding styles sane, we want the order of loaded
+    // sheets to be something like this:
+    //   global.css
+    //   *-theme.css (the file loaded here)
+    //   document-specific-sheet.css
+    // See Bug 1582786 / https://phabricator.services.mozilla.com/D46530#inline-284756.
+    const globalSheet = doc.head.querySelector(
+      "link[href='chrome://global/skin/global.css']"
+    );
+    if (globalSheet) {
+      globalSheet.after(styleSheet);
+    } else {
+      doc.head.prepend(styleSheet);
+    }
+    return { styleSheet, loadPromise };
+  }
+
+  const styleSheet = doc.createProcessingInstruction(
     "xml-stylesheet",
-    styleSheetAttr
+    `href="${url}" type="text/css"`
   );
-  const loadPromise = new Promise((resolve, reject) => {
-    function onload() {
-      styleSheet.removeEventListener("load", onload);
-      styleSheet.removeEventListener("error", onerror);
-      resolve();
-    }
-    function onerror() {
-      styleSheet.removeEventListener("load", onload);
-      styleSheet.removeEventListener("error", onerror);
-      reject("Failed to load theme file " + url);
-    }
-
-    styleSheet.addEventListener("load", onload);
-    styleSheet.addEventListener("error", onerror);
-  });
-  xulDocument.insertBefore(styleSheet, xulDocument.documentElement);
+  const loadPromise = stylesheetLoadPromise(styleSheet);
+  doc.insertBefore(styleSheet, doc.documentElement);
   return { styleSheet, loadPromise };
 }
 
 exports.appendStyleSheet = appendStyleSheet;
--- a/devtools/client/shared/test/doc_options-view.xul
+++ b/devtools/client/shared/test/doc_options-view.xul
@@ -1,13 +1,13 @@
 <?xml version="1.0"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/content/shared/widgets/widgets.css" type="text/css"?>
 <!DOCTYPE window []>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
     <popupset id="options-popupset">
         <menupopup id="options-menupopup" position="before_end">
--- a/devtools/client/storage/index.xul
+++ b/devtools/client/storage/index.xul
@@ -1,13 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - 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/. -->
-<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/content/shared/widgets/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/widgets.css" type="text/css"?>
 <?xml-stylesheet href="chrome://devtools/skin/storage.css" type="text/css"?>
 <?xml-stylesheet href="resource://devtools/client/shared/components/SidebarToggle.css" type="text/css"?>
 
 <!DOCTYPE window [
   <!ENTITY % storageDTD SYSTEM "chrome://devtools/locale/storage.dtd">
   %storageDTD;
--- a/devtools/client/webconsole/index.html
+++ b/devtools/client/webconsole/index.html
@@ -5,17 +5,17 @@
 <html dir=""
       id="devtools-webconsole"
       windowtype="devtools:webconsole"
       width="900" height="350"
       persist="screenX screenY width height sizemode">
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
     <link rel="localization" href="toolkit/main-window/editmenu.ftl"/>
-    <link rel="stylesheet" href="chrome://global/skin/"/>
+    <link rel="stylesheet" href="chrome://global/skin/global.css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/widgets.css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/webconsole.css"/>
     <link rel="stylesheet" href="chrome://devtools/skin/components-frame.css"/>
     <link rel="stylesheet" href="resource://devtools/client/shared/components/SmartTrace.css"/>
     <link rel="stylesheet" href="resource://devtools/client/shared/components/reps/reps.css"/>
     <link rel="stylesheet" href="resource://devtools/client/shared/components/tabs/Tabs.css"/>
     <link rel="stylesheet" href="resource://devtools/client/shared/components/NotificationBox.css"/>
     <link rel="stylesheet" href="resource://devtools/client/shared/components/splitter/GridElementResizer.css"/>