chrome/content/ui/firstRun.html - Issue 11533106: Prepared adblockplus for Safari

Unified Diff: chrome/content/ui/firstRun.html

Issue 11533106: Prepared adblockplus for Safari (Closed)

Patch Set: Fixed broken path to CSS for firefox Created Sept. 6, 2013, 5:18 p.m.

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View side-by-side diff with in-line comments

Index: chrome/content/ui/firstRun.html

===================================================================

--- a/chrome/content/ui/firstRun.html

+++ b/chrome/content/ui/firstRun.html

@@ -20,8 +20,19 @@

<head>

- <link type="text/css" href="/skin/firstRun.css" rel="stylesheet"/>

+

+ <link type="text/css" href="../../skin/firstRun.css" rel="stylesheet"/>

+

+ <link type="text/css" href="skin/firstRun.css" rel="stylesheet"/>

Wladimir Palant 2013/09/10 06:53:46 I'm not particularly happy with using random relat

Sebastian Noack 2013/09/10 10:56:29 Yes, there isn't an other solution (for safari). H

+ <script type="text/javascript" src="ext.js"></script>

+ <script type="text/javascript" src="lib/compat.js"></script>

+ <script type="text/javascript" src="lib/io.js"></script>

+ <script type="text/javascript" src="lib/info.js"></script>

+ <script type="text/javascript" src="lib/adblockplus.js"></script>

Wladimir Palant 2013/09/10 06:53:46 Loading the core code into the page isn't going to

Sebastian Noack 2013/09/10 10:56:29 Unfortunately you are right. I didn't thought abou

On 2013/09/10 06:53:46, Wladimir Palant wrote: > Loading the core code into the page isn't going to do you any good - it will > slow down the loading of this page significantly but you will be operating on a > copy of the data, not the data used by the rest of the extension. This cannot > possibly work correctly. > > A proper solution requires this page to communicate with the rest of the > extension. From the documentation it isn't clear whether a page will be able to > do that by itself, you might need to inject a content script into it. > > You should be able to abuse safari.self.tab.canLoad() to do synchronous > messaging. require() should work by sending a synchronous message to the global > page and getting a list of methods and properties back. Using that list it > should construct a wrapper: > > * Property getters use synchronous messaging to retrieve their value from the > global page (any non-primitive values should again be resolved into wrappers). > * Property setters use synchronous messaging to set the value in the global > page. > * Method calls are forwarded to the global page via synchronous messaging, their > result is returned (any non-primitive values should again be resolved into > wrappers). > > Note that the global page will need to keep a list of objects that wrappers have > been created for - otherwise it won't know which object to call a method on. We > cannot really know when a wrapper will stop being used, notifying the global > page in the "unload" event that it can clear that list should be enough to avoid > a memory leak however. > > From what I can tell, FilterNotifier will be the only object needing special > handling. Wrapping it won't do any good, you will need to reimplement it for > this page. So FilterNotifier.addListener() will ask the global page to register > a listener on its end and send an async message to the page whenever it is > called. > > All in all, not exactly a simple approach, the wrapper logic won't really be > trivial. However, I don't see how we are going to avoid doing this.

Unfortunately you are right. I didn't thought about it before, well enough. The first run page, currently operates on a local copy of the data, and therefore the UI switches in the firstRun page wouldn't do anything. The same problem exists for the options page. And yes the only solution will be to implement a proxy interface that uses synchronous messaging. I got this idea before, but I tried to avoid it so far, because of this will become really complex, tricky and ugly. But it seems that we don't have any other option, now. :(

</head>

@@ -32,8 +43,8 @@

</header>

+ <p id="legacySafariWarning" class="i18n_firstRun_legacySafariWarning" hidden="true"></p>

« no previous file with comments | « no previous file | chrome/content/ui/firstRun.js » ('j') | chrome/content/ui/firstRun.js » ('J')