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

Side by Side 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.

Left:
Right:

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

Jump to:

View unified diff | Download patch

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <!--	2 <!--

3 - This file is part of Adblock Plus <http://adblockplus.org/>,	3 - This file is part of Adblock Plus <http://adblockplus.org/>,

4 - Copyright (C) 2006-2013 Eyeo GmbH	4 - Copyright (C) 2006-2013 Eyeo GmbH

5 -	5 -

6 - Adblock Plus is free software: you can redistribute it and/or modify	6 - Adblock Plus is free software: you can redistribute it and/or modify

7 - it under the terms of the GNU General Public License version 3 as	7 - it under the terms of the GNU General Public License version 3 as

8 - published by the Free Software Foundation.	8 - published by the Free Software Foundation.

9 -	9 -

10 - Adblock Plus is distributed in the hope that it will be useful,	10 - Adblock Plus is distributed in the hope that it will be useful,

11 - but WITHOUT ANY WARRANTY; without even the implied warranty of	11 - but WITHOUT ANY WARRANTY; without even the implied warranty of

12 - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the	12 - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

13 - GNU General Public License for more details.	13 - GNU General Public License for more details.

14 -	14 -

15 - You should have received a copy of the GNU General Public License	15 - You should have received a copy of the GNU General Public License

16 - along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.	16 - along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.

17 -->	17 -->

18	18

19 <html>	19 <html>

20 <head>	20 <head>

21 <title class="i18n_firstRun_title"></title>	21 <title class="i18n_firstRun_title"></title>

22 <meta charset="utf-8">	22 <meta charset="utf-8">

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

	24 <!-- only for firefox -->

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

	26

	27 <!-- only required for the ports -->

	28 <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 I'm not particularly happy with using random relative paths to refer to skin files and assuming that these relative paths will only point somewhere on one platform. If there is no other solution (and seeing Safari documentation explicitly discourage host-relative URLs there probably isn't) I would prefer keeping the paths as they are and convert them automatically in the Safari packager. Note: the comment claiming that we were already loading non-existing files here is wrong. utils.js does exist in Firefox: https://hg.adblockplus.org/adblockplus/file/b823f710a2c7/chrome/content/ui/ut.... The adblockpluschrome repository merely contains a different version of that file. Sebastian Noack 2013/09/10 10:56:29 Yes, there isn't an other solution (for safari). H Show quoted text On 2013/09/10 06:53:46, Wladimir Palant wrote: > I'm not particularly happy with using random relative paths to refer to skin > files and assuming that these relative paths will only point somewhere on one > platform. If there is no other solution (and seeing Safari documentation > explicitly discourage host-relative URLs there probably isn't) I would prefer > keeping the paths as they are and convert them automatically in the Safari > packager. Yes, there isn't an other solution (for safari). Host relative paths lead to an error and the resource doesn't load. I tried it out. I don't like the idea of converting the HTML in the packager, because of it might easily break when the HTML file is changed without the conversion in mind. However it still might be the best option we have. :( Show quoted text > Note: the comment claiming that we were already loading non-existing files here > is wrong. utils.js does exist in Firefox: > https://hg.adblockplus.org/adblockplus/file/b823f710a2c7/chrome/content/ui/ut.... > The adblockpluschrome repository merely contains a different version of that > file. Oh, you are right, my fault.
	29 <script type="text/javascript" src="ext.js"></script>

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

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

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

	33 <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 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. Sebastian Noack 2013/09/10 10:56:29 Unfortunately you are right. I didn't thought abou Show quoted text 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. :(
24 <script type="text/javascript" src="utils.js"></script>	34 <script type="text/javascript" src="utils.js"></script>

	35

25 <script type="text/javascript" src="i18n.js"></script>	36 <script type="text/javascript" src="i18n.js"></script>

26 <script type="text/javascript" src="firstRun.js"></script>	37 <script type="text/javascript" src="firstRun.js"></script>

27 </head>	38 </head>

28 <body>	39 <body>

29 <div id="shade" class="arrow"></div>	40 <div id="shade" class="arrow"></div>

30 <div id="content">	41 <div id="content">

31 <header>	42 <header>

32 <h1 id="title-main" class="i18n_firstRun_title"></h1>	43 <h1 id="title-main" class="i18n_firstRun_title"></h1>

33 </header>	44 </header>

34	45

	46 <p id="legacySafariWarning" class="i18n_firstRun_legacySafariWarning" hidd en="true"></p>

35 <p id="dataCorruptionWarning" class="i18n_firstRun_dataCorruptionWarning" hidden="true"></p>	47 <p id="dataCorruptionWarning" class="i18n_firstRun_dataCorruptionWarning" hidden="true"></p>

36

37 <p id="acceptableAdsExplanation" class="i18n_firstRun_acceptableAdsExplana tion"></p>	48 <p id="acceptableAdsExplanation" class="i18n_firstRun_acceptableAdsExplana tion"></p>

38	49

39 <h2 id="features-title" class="i18n_firstRun_features"></h2>	50 <h2 id="features-title" class="i18n_firstRun_features"></h2>

40	51

41 <ul id="features">	52 <ul id="features">

42 <li id="feature-malware" class="feature">	53 <li id="feature-malware" class="feature">

43 <div class="feature-image"></div>	54 <div class="feature-image"></div>

44 <div class="feature-description">	55 <div class="feature-description">

45 <p class="feature-text">	56 <p class="feature-text">

46 <span class="i18n_firstRun_feature_malware feature-title"></span> <span class="i18n_firstRun_feature_malware_description"></span>	57 <span class="i18n_firstRun_feature_malware feature-title"></span> <span class="i18n_firstRun_feature_malware_description"></span>

(...skipping 47 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
94 </li>	105 </li>

95 </ul>	106 </ul>

96 </footer>	107 </footer>

97 </div>	108 </div>

98	109

99 <div id="glass-pane">	110 <div id="glass-pane">

100 <iframe id="share-popup" scrolling="no"></iframe>	111 <iframe id="share-popup" scrolling="no"></iframe>

101 </div>	112 </div>

102 </body>	113 </body>

103 </html>	114 </html>

OLD	NEW

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