Issue 3532 - Generate animation images at runtime

safari/ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 136 matching lines @@
     }
   };
 
   safari.application.addEventListener("activate", function(event)
   {
     // this event is also dispatched on windows that got focused. But we
     // are only interested in tabs, which became active in their window.
     if (!(event.target instanceof SafariBrowserTab))
       return;
 
+    var activeTab = event.target.browserWindow.activeTab;
+    var pageId = addPage(activeTab, activeTab.url, false);

[Sebastian Noack, 2016/01/26 16:12:38]

Add page registers a new page (in our data structure) when reported by our
content script on page load, or when discovering existing pages during extension
intilization. When a tab becomes active later it is already known. Even worse
calling addPage() again will generate a new page ID which messes up the internal
state.

The page you are looking for should be tab._visiblePage.

[kzar, 2016/01/26 22:59:30]

Acknowledged.

+    ext.pages.onActivated._dispatch(pages[pageId]);
+
     // update the toolbar item for the page visible in the tab that just
     // became active. If we can't find that page (e.g. when a page was
     // opened in a new tab, and our content script didn't run yet), the
     // toolbar item of the window, is reset to its intial configuration.
     updateToolbarItemForPage(event.target._visiblePage, event.target.browserWindow);
   }, true);
 
 
   /* Pages */
 
   var pages = Object.create(null);
   var pageCounter = 0;
 
   var Page = function(id, tab, url)
   {
     this._id = id;
     this._tab = tab;
-    this._frames = [{url: new URL(url), parent: null}];
+    this._frames = [{url: new URL(url || "about:blank"), parent: null}];

[Sebastian Noack, 2016/01/26 16:12:37]

This change seems to be unrelated.

[Sebastian Noack, 2016/01/26 22:44:50]

It seems that you forgot to reply here. At least this change is still in the
latest patch set. And I'm still wondering in which scenario url will be empty,
and how this is related to the change here.

[kzar, 2016/01/26 22:59:30]

Whoops, you're right. Somehow in the past I was seeing warnings which this
resolved. I've re-tested the animations after removing this change however and
they're working fine. I guess there was a bug elsewhere previously.

 
     if (tab.page)
       this._messageProxy = new ext._MessageProxy(tab.page);
     else
       // while the new tab page is shown on Safari 7, the 'page' property
       // of the tab is undefined, and we can't send messages to that page
       this._messageProxy = {
         handleRequest: function() {},
         handleResponse: function() {},
         sendMessage: function() {}
@@ skipping 101 matching lines @@
         if ("active" in info && info.active != isPageActive(page))
           continue;
         if ("lastFocusedWindow" in info && info.lastFocusedWindow != (win == safari.application.activeBrowserWindow))
           continue;
 
         matchedPages.push(page);
       };
 
       callback(matchedPages);
     },
-    onLoading: new ext._EventTarget()
+    onLoading: new ext._EventTarget(),
+    onActivated: new ext._EventTarget(),
   };
 
   safari.application.addEventListener("close", function(event)
   {
     // this event is dispatched on closing windows and tabs. However when a
     // window is closed, it is first dispatched on each tab in the window and
     // then on the window itself. But we are only interested in closed tabs.
     if (!(event.target instanceof SafariBrowserTab))
       return;
 
@@ skipping 430 matching lines @@
         tab.activate();
         if (callback)
           callback(page);
         return;
       }
     }
 
     ext.pages.open(optionsUrl, callback);
   };
 })();