Bug 1271049 - Fix showing alternative styles from <style> elements and make the test actually test things. r=mconley, a=sledru
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 09 May 2016 14:35:54 +0100
changeset 333062 4fd7d6aeb9b4052ca530813086d0117554175942
parent 333061 fd75a1f393fe0173accbd1f14b14379b4266c503
child 333063 8ceaff1404be27309bf6fa0e127f9bd9ce36a3fc
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley, sledru
bugs1271049
milestone48.0a2
Bug 1271049 - Fix showing alternative styles from <style> elements and make the test actually test things. r=mconley, a=sledru MozReview-Commit-ID: BD7eQl5iNaJ
browser/base/content/tab-content.js
browser/base/content/test/general/browser_page_style_menu.js
browser/base/content/test/general/page_style_sample.html
--- a/browser/base/content/tab-content.js
+++ b/browser/base/content/tab-content.js
@@ -502,34 +502,37 @@ var PageStyleHandler = {
         let mediaQueryList = currentStyleSheet.media.mediaText;
         if (!content.matchMedia(mediaQueryList).matches) {
           continue;
         }
       }
 
       let URI;
       try {
-        URI = Services.io.newURI(currentStyleSheet.href, null, null);
+        if (!currentStyleSheet.ownerNode ||
+            // special-case style nodes, which have no href
+            currentStyleSheet.ownerNode.nodeName.toLowerCase() != "style") {
+          URI = Services.io.newURI(currentStyleSheet.href, null, null);
+        }
       } catch(e) {
         if (e.result != Cr.NS_ERROR_MALFORMED_URI) {
           throw e;
         }
+        continue;
       }
 
-      if (URI) {
-        // We won't send data URIs all of the way up to the parent, as these
-        // can be arbitrarily large.
-        let sentURI = URI.scheme == "data" ? null : URI.spec;
+      // We won't send data URIs all of the way up to the parent, as these
+      // can be arbitrarily large.
+      let sentURI = (!URI || URI.scheme == "data") ? null : URI.spec;
 
-        result.push({
-          title: currentStyleSheet.title,
-          disabled: currentStyleSheet.disabled,
-          href: sentURI,
-        });
-      }
+      result.push({
+        title: currentStyleSheet.title,
+        disabled: currentStyleSheet.disabled,
+        href: sentURI,
+      });
     }
 
     return result;
   },
 };
 PageStyleHandler.init();
 
 // Keep a reference to the translation content handler to avoid it it being GC'ed.
--- a/browser/base/content/test/general/browser_page_style_menu.js
+++ b/browser/base/content/test/general/browser_page_style_menu.js
@@ -1,67 +1,97 @@
-function test() {
-  waitForExplicitFinish();
+"use strict";
 
-  var tab = gBrowser.addTab();
-  gBrowser.selectedTab = tab;
-  tab.linkedBrowser.addEventListener("pageshow", function () {
-    tab.linkedBrowser.removeEventListener("pageshow", arguments.callee, true);
-    checkPageStyleMenu();
-  }, true);
-  let rootDir = getRootDirectory(gTestPath);
-  content.location = rootDir + "page_style_sample.html";
+/**
+ * Stylesheets are updated for a browser after the pageshow event.
+ * This helper function returns a Promise that waits for that pageshow
+ * event, and then resolves on the next tick to ensure that gPageStyleMenu
+ * has had a chance to update the stylesheets.
+ *
+ * @param browser
+ *        The <xul:browser> to wait for.
+ * @return Promise
+ */
+function promiseStylesheetsUpdated(browser) {
+  return ContentTask.spawn(browser, { PAGE }, function*(args) {
+    return new Promise((resolve) => {
+      addEventListener("pageshow", function onPageShow(e) {
+        if (e.target.location == args.PAGE) {
+          removeEventListener("pageshow", onPageShow);
+          content.setTimeout(resolve, 0);
+        }
+      });
+    })
+  });
 }
 
-function checkPageStyleMenu() {
-  var menupopup = document.getElementById("pageStyleMenu")
-                          .getElementsByTagName("menupopup")[0];
+const PAGE = "http://example.com/browser/browser/base/content/test/general/page_style_sample.html";
+
+/*
+ * Test that the right stylesheets do (and others don't) show up
+ * in the page style menu.
+ */
+add_task(function*() {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank", false);
+  let browser = tab.linkedBrowser;
+  yield BrowserTestUtils.loadURI(browser, PAGE);
+  yield promiseStylesheetsUpdated(browser);
+
+  let menupopup = document.getElementById("pageStyleMenu").menupopup;
   gPageStyleMenu.fillPopup(menupopup);
 
   var items = [];
   var current = menupopup.getElementsByTagName("menuseparator")[0];
   while (current.nextSibling) {
     current = current.nextSibling;
     items.push(current);
   }
 
-  var validLinks = 0;
-  Array.forEach(content.document.getElementsByTagName("link"), function (link) {
-    var title = link.getAttribute("title");
-    var rel = link.getAttribute("rel");
-    var media = link.getAttribute("media");
-    var idstring = "link " + (title ? title : "without title and") +
-                   " with rel=\"" + rel + "\"" +
-                   (media ? " and media=\"" + media + "\"" : "");
+  items = items.map(el => ({
+    label: el.getAttribute("label"),
+    checked: el.getAttribute("checked") == "true",
+  }));
 
-    var item = items.filter(item => item.getAttribute("label") == title);
-    var found = item.length == 1;
-    var checked = found && (item[0].getAttribute("checked") == "true");
+  let validLinks = yield ContentTask.spawn(gBrowser.selectedBrowser, items, function(items) {
+    let validLinks = 0;
+    Array.forEach(content.document.querySelectorAll("link, style"), function (el) {
+      var title = el.getAttribute("title");
+      var rel = el.getAttribute("rel");
+      var media = el.getAttribute("media");
+      var idstring = el.nodeName + " " + (title ? title : "without title and") +
+                     " with rel=\"" + rel + "\"" +
+                     (media ? " and media=\"" + media + "\"" : "");
+
+      var item = items.filter(item => item.label == title);
+      var found = item.length == 1;
+      var checked = found && item[0].checked;
 
-    switch (link.getAttribute("data-state")) {
-      case "0":
-        ok(!found, idstring + " does not show up in page style menu");
-        break;
-      case "0-todo":
-        validLinks++;
-        todo(!found, idstring + " should not show up in page style menu");
-        ok(!checked, idstring + " is not selected");
-        break;
-      case "1":
-        validLinks++;
-        ok(found, idstring + " shows up in page style menu");
-        ok(!checked, idstring + " is not selected");
-        break;
-      case "2":
-        validLinks++;
-        ok(found, idstring + " shows up in page style menu");
-        ok(checked, idstring + " is selected");
-        break;
-      default:
-        throw "data-state attribute is missing or has invalid value";
-    }
+      switch (el.getAttribute("data-state")) {
+        case "0":
+          ok(!found, idstring + " should not show up in page style menu");
+          break;
+        case "0-todo":
+          validLinks++;
+          todo(!found, idstring + " should not show up in page style menu");
+          ok(!checked, idstring + " should not be selected");
+          break;
+        case "1":
+          validLinks++;
+          ok(found, idstring + " should show up in page style menu");
+          ok(!checked, idstring + " should not be selected");
+          break;
+        case "2":
+          validLinks++;
+          ok(found, idstring + " should show up in page style menu");
+          ok(checked, idstring + " should be selected");
+          break;
+        default:
+          throw "data-state attribute is missing or has invalid value";
+      }
+    });
+    return validLinks;
   });
 
-  is(validLinks, items.length, "all valid links found");
+  ok(items.length, "At least one item in the menu");
+  is(items.length, validLinks, "all valid links found");
 
-  gBrowser.removeCurrentTab();
-  finish();
-}
+  yield BrowserTestUtils.removeTab(tab);
+});
--- a/browser/base/content/test/general/page_style_sample.html
+++ b/browser/base/content/test/general/page_style_sample.html
@@ -30,11 +30,12 @@
     <link data-state="0" href="404.css" title="20" rel="alternate stylesheet" media="all  screen">
     <link data-state="0" href="404.css" title="21" rel="alternate stylesheet" media="foo">
     <link data-state="0" href="404.css" title="22" rel="alternate stylesheet" media="allscreen">
     <link data-state="0" href="404.css" title="23" rel="alternate stylesheet" media="_all">
     <link data-state="0" href="404.css" title="24" rel="alternate stylesheet" media="not screen">
     <link data-state="1" href="404.css" title="25" rel="alternate stylesheet" media="only screen">
     <link data-state="1" href="404.css" title="26" rel="alternate stylesheet" media="screen and (min-device-width: 1px)">
     <link data-state="0" href="404.css" title="27" rel="alternate stylesheet" media="screen and (max-device-width: 1px)">
+    <style data-state="1" title="28">/* some more styles */</style>
   </head>
   <body></body>
 </html>