Issue 581 - Fixed element hiding/collapsing in anonymous frames on Chrome

include.preload.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 var SELECTOR_GROUP_SIZE = 20;
 
-// Converts relative to absolute URL
-// e.g.: foo.swf on http://example.com/whatever/bar.html
-//  -> http://example.com/whatever/foo.swf
-function relativeToAbsoluteUrl(url)
-{
-  // If URL is already absolute, don't mess with it
-  if (!url || /^[\w\-]+:/i.test(url))
-    return url;
-
-  // Leading / means absolute path
-  // Leading // means network path
-  if (url[0] == '/')
-  {
-    if (url[1] == '/')
-      return document.location.protocol + url;
-    else
-      return document.location.protocol + "//" + document.location.host + url;
-  }
-
-  // Remove filename and add relative URL to it
-  var base = document.baseURI.match(/.+\//);
-  if (!base)
-    return document.baseURI + "/" + url;
-  return base[0] + url;
-}
-
 var typeMap = {
   "img": "IMAGE",
   "input": "IMAGE",
   "audio": "MEDIA",
   "video": "MEDIA",
   "frame": "SUBDOCUMENT",
   "iframe": "SUBDOCUMENT"
 };
 
 function checkCollapse(element)

[Wladimir Palant, 2014/05/30 11:45:17]

Please move this function and the typeMap declaration back where they originally
were. I cannot review it like this. In general, please always commit (and
review) cosmetic and functional changes separately.

[Sebastian Noack, 2014/05/30 12:31:11]

Done.

 {
   var tag = element.localName;
   if (tag in typeMap)
   {
     // This element failed loading, did we block it?
     var url = element.src;
     if (!url)
       return;
 
     ext.backgroundPage.sendMessage(
@@ skipping 11 matching lines @@
           if (tag == "frame")
             element.style.setProperty("visibility", "hidden", "important");
           else
             element.style.setProperty("display", "none", "important");
         }
       }
     );
   }
 }
 
-function checkExcpetionKey()

[Wladimir Palant, 2014/05/30 11:45:17]

Excpetion => Exception

[Sebastian Noack, 2014/05/30 12:31:11]

Done.

+function checkExceptionKey()
 {
   var attr = document.documentElement.getAttribute("data-adblockkey");
   if (attr)
     ext.backgroundPage.sendMessage({type: "add-key-exception", token: attr});
 }
 
+function hasInlineURL(element, attribute)
+{
+  var value = element.getAttribute(attribute);
+  return value == null || /^\s*(javascript:|about:|$)/i.test(value);
+}
+
+function isInlineFrame(element)
+{
+  switch (element.localName)
+  {
+    case "iframe":
+      return hasInlineURL(element, "src") || element.hasAttribute("srcdoc");
+    case "frame":
+      return hasInlineURL(element, "src");
+    case "object":
+      return hasInlineURL(element, "data") && element.contentDocument;
+    default:
+      return false;
+  }
+}
+
+// Converts relative to absolute URL
+// e.g.: foo.swf on http://example.com/whatever/bar.html
+//  -> http://example.com/whatever/foo.swf
+function relativeToAbsoluteUrl(url)
+{
+  // If URL is already absolute, don't mess with it
+  if (!url || /^[\w\-]+:/i.test(url))
+    return url;
+
+  // Leading / means absolute path
+  // Leading // means network path
+  if (url[0] == '/')
+  {
+    if (url[1] == '/')
+      return document.location.protocol + url;
+    else
+      return document.location.protocol + "//" + document.location.host + url;
+  }
+
+  // Remove filename and add relative URL to it
+  var base = document.baseURI.match(/.+\//);
+  if (!base)
+    return document.baseURI + "/" + url;
+  return base[0] + url;
+}
+
 function init(document)
 {
+  var canUseShadow = "webkitCreateShadowRoot" in document.documentElement;
+  var fixInlineFrames = false;
+
+  var match = navigator.userAgent.match(/\bChrome\/(\d+)/);
+  if (match)
+  {
+    var chromeVersion = parseInt(match[1]);
+
+    // the <shadow> element is ignored in Chrome 32 (#309). Also Chrome 31-33
+    // crashes in some situations on some pages when using shadow DOM (#498).
+    // So we must not use Shadow DOM on those versions of Chrome.
+    if (chromeVersion >= 31 && chromeVersion <= 33)
+      canUseShadow = false;
+
+    // prior to Chrome 37, content scripts don't run on about:blank
+    // and about:srcdoc. So we have to apply element hiding and collapsing
+    // from the parent frame, when inline frames are loaded.
+    if (chromeVersion < 37)
+      fixInlineFrames = true;
+  }
+
   // use Shadow DOM if available to don't mess with web pages that
-  // rely on the order of their own <style> tags. However
-  // the <shadow> element is broken in some Chrome 32 builds (#309)
-  //
-  // also Chrome 31-33 crashes in some situations on some pages when using
-  // ShadowDOM, e.g. when pressing tab key on Wikipedia and Facebook (#498)
-  //
-  // also we must not create the shadow root in the response callback passed
-  // to sendMessage(), otherwise Chrome breaks some websites (#450)
-  var shadow = null;
-  if ("webkitCreateShadowRoot" in document.documentElement && !/\bChrome\/3[1-3]\b/.test(navigator.userAgent))
-  {
-    shadow = document.documentElement.webkitCreateShadowRoot();
+  // rely on the order of their own <style> tags (#309). However we
+  // must not create the shadow root in the response callback passed
+  // to sendMessage(), otherwise Chrome breaks some websites (#450).
+  if (canUseShadow)
+  {
+    var shadow = document.documentElement.webkitCreateShadowRoot();
     shadow.appendChild(document.createElement("shadow"));
   }
 
   // Sets the currently used CSS rules for elemhide filters
   var setElemhideCSSRules = function(selectors)
   {
     if (selectors.length == 0)
       return;
 
     var style = document.createElement("style");
     style.setAttribute("type", "text/css");
 
-    if (shadow)
+    if (canUseShadow)
     {
       shadow.appendChild(style);
 
       try
       {
         document.querySelector("::content");
 
         for (var i = 0; i < selectors.length; i++)
           selectors[i] = "::content " + selectors[i];
       }
@@ skipping 33 matching lines @@
     setRules();
   };
 
   document.addEventListener("error", function(event)
   {
     checkCollapse(event.target);
   }, true);
 
   document.addEventListener("load", function(event)
   {
-    if (/^i?frame$/.test(event.target.localName))
-    {
-      checkCollapse(event.target);
-
-      // Chrome doesn't run our content script on anonymous frames.
-      // So we have to apply element hiding and collapsing from the
-      // parent frame, when an anonymous sub frame loaded.
-      if (/^(javascript:|$)/.test(event.target.src) && /\bChrome\//.test(navigator.userAgent))
-      {
-        var contentDocument = event.target.contentDocument;
-
-        if (contentDocument.documentElement instanceof HTMLElement)
-        {
-          init(contentDocument);
-
-          for (tagName in typeMap)
-            Array.prototype.forEach.call(contentDocument.getElementsByTagName(tagName), checkCollapse);
-        }
-      }
+    var element = event.target;
+
+    if (/^i?frame$/.test(element.localName))
+      checkCollapse(element);
+
+    if (fixInlineFrames && isInlineFrame(element))
+    {
+      init(element.contentDocument);
+
+      for (var tagName in typeMap)
+        Array.prototype.forEach.call(element.contentDocument.getElementsByTagName(tagName), checkCollapse);
     }
   }, true);
 
   ext.backgroundPage.sendMessage({type: "get-selectors"}, setElemhideCSSRules);
 }
 
 if (document.documentElement instanceof HTMLElement)

[Wladimir Palant, 2014/05/30 11:45:17]

Why did you remove the failsafe? It was added back when Chrome 18 was the *new*
version, the supported Chrome versions might also show the same behavior.

[Sebastian Noack, 2014/05/30 12:31:11]

I tested in Chrome 34 and Chrome 30, and it seems not to be necessary anymore.
Also AdBlock (which claims to be still compatible down to Chrome 23) doesn't do
that (anymore). So I think it is safe to get rid of that check now.

However as discussed on IRC, I don't think that we have to merge this for the
upcoming release, but should heavily test the changes of this patch, regardless
whether we remove that check or not.

 {
-  checkExcpetionKey();
+  checkExceptionKey();
   init(document);

[Wladimir Palant, 2014/05/30 11:45:17]

Why pass in the document which is a global variable anyway?

[Sebastian Noack, 2014/05/30 12:31:11]

It is not always called with the global document. See above, where the
contentDocument of the frame is passed.

 }