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]);
+    let visiblePage = event.target._visiblePage;
+    if (visiblePage)
+      ext.pages.onActivated._dispatch(visiblePage);
 
     // 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);
+    updateToolbarItemForPage(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 || "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.

+    this._frames = [{url: new URL(url), parent: null}];
 
     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 553 matching lines @@
         tab.activate();
         if (callback)
           callback(page);
         return;
       }
     }
 
     ext.pages.open(optionsUrl, callback);
   };
 })();