Issue 1592 - Fixed frame collapsing on Safari

safari/ext/content.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
@@ skipping 86 matching lines @@
         category: "webRequest",
         url: url,
         type: type,
         pageId: documentInfo.pageId,
         frameId: documentInfo.frameId
       }
     ))
     {
       event.preventDefault();
 
-      // Safari doesn't dispatch an "error" event when preventing an element
-      // from loading by cancelling the "beforeload" event. Starting with

[kzar, 2014/11/24 16:54:50]

Nitpick: This comment doesn't read very well starting at the second sentence.
Maybe rewrite?

[Sebastian Noack, 2014/11/24 17:13:20]

Can you explain why it doesn't read well in your opinion? How would you improve
it?

[kzar, 2014/11/25 11:27:33]

The part "...it not even dispatches..." would be more idiomatically written as
"...it doesn't even dispatch..." but either fragment doesn't fit in the sentence
that well. How about this?

"Safari doesn't dispatch all the expected events for elements that have been
prevented from loading by having their "beforeload" event cancelled. "error"
events are never dispatched and with versions of Safari 8 and up "load" events
are also not dispatched. We need to dispatch those events manually here to avoid
breaking element collapsing and therefore the pages that rely on it."

[Sebastian Noack, 2014/11/25 11:48:38]

I used a sligthly modified version of your text. I changed following:

1. I prefer to explicitly mentioning the relationship between "load" events and
frames.
2. Some pages are not only broken in the sense that element collapsing won't
work, but might also break in a much more fatal way, when they rely on those
events being dispatched. I remember an issue with a website which weren't
functional until their "load" or "error" event listener were called.

[kzar, 2014/11/25 12:06:53]

Cool, reads great

-      // Safari 8, it not even dispatches a "load" event for cancelled frames.
-      // So we have to dispatch it manually. Otherwise, element collapsing
-      // wouldn't work, and pages that rely on those events break.
-      if (type != "sub_frame" || parseInt(navigator.userAgent.match(/\bVersion\/(\d+)/)[1], 10) >= 8)

[kzar, 2014/11/24 16:54:50]

What if the user is using a version < 8, shouldn't the error event still be
dispatched?

[Sebastian Noack, 2014/11/24 17:13:20]

Yes, and it is. However, error events should only be dispatched for elements
other than frames, in which case the check passes.

[kzar, 2014/11/25 11:27:33]

Ah I see, sorry.

-      {
-        setTimeout(function()
-        {
-          var evt = document.createEvent("Event");
-          evt.initEvent(eventName);
-          event.target.dispatchEvent(evt);
-        }, 0);
-      }
+      // Safari doesn't dispatch the expected events for elements that have been
+      // prevented from loading by having their "beforeload" event cancelled.
+      // That is a "load" event for blocked frames, and an "error" event for
+      // other blocked elements. We need to dispatch those events manually here
+      // to avoid breaking element collapsing and pages that rely on those events.
+      setTimeout(function()
+      {
+        var evt = document.createEvent("Event");
+        evt.initEvent(eventName);
+        event.target.dispatchEvent(evt);
+      }, 0);
     }
   }, true);
 
 
   /* Context menus */
 
   document.addEventListener("contextmenu", function(event)
   {
     var element = event.srcElement;
     safari.self.tab.setContextMenuEventUserInfo(event, {
@@ skipping 294 matching lines @@
             messageProxy.handleResponse(event.message);
             break;
           case "proxyCallback":
             backgroundPageProxy.handleCallback(event.message);
             break;
         }
       }
     }
   });
 })();