Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: firstRun.js

Issue 6180766664884224: Issue 1663 - Made first-run page use an asynchronous messaging protocol (Closed)
Left Patch Set: Created Dec. 16, 2014, 2:08 p.m.
Right Patch Set: Adressed comments Created Dec. 18, 2014, 7:30 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « ext/content.js ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <http://adblockplus.org/>, 2 * This file is part of Adblock Plus <http://adblockplus.org/>,
3 * Copyright (C) 2006-2014 Eyeo GmbH 3 * Copyright (C) 2006-2014 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 25 matching lines...) Expand all
36 { 36 {
37 feature: "tracking", 37 feature: "tracking",
38 homepage: "https://easylist.adblockplus.org/", 38 homepage: "https://easylist.adblockplus.org/",
39 title: "EasyPrivacy", 39 title: "EasyPrivacy",
40 url: "https://easylist-downloads.adblockplus.org/easyprivacy.txt" 40 url: "https://easylist-downloads.adblockplus.org/easyprivacy.txt"
41 } 41 }
42 ]; 42 ];
43 43
44 function getDocLink(link, callback) 44 function getDocLink(link, callback)
45 { 45 {
46 ext.backgroundPage.sendMessage({type: "app.doclink", args: [link]}, callback ); 46 ext.backgroundPage.sendMessage({
Thomas Greiner 2014/12/18 10:17:48 Isn't that exactly what we didn't want to do, send
Wladimir Palant 2014/12/18 19:31:35 Sure, but that's more relevant for the scenarios w
47 type: "app.get",
48 what: "doclink",
49 link: link
50 }, callback);
47 } 51 }
48 52
49 function onDOMLoaded() 53 function onDOMLoaded()
50 { 54 {
51 // Set up URLs 55 // Set up URLs
52 getDocLink("donate", function(link) 56 getDocLink("donate", function(link)
53 { 57 {
54 E("donate").href = link; 58 E("donate").href = link;
55 }); 59 });
56 60
57 getDocLink("contributors", function(link) 61 getDocLink("contributors", function(link)
58 { 62 {
59 E("contributors").href = link; 63 E("contributors").href = link;
60 }); 64 });
61 65
62 getDocLink("acceptable_ads_criteria", function(link) 66 getDocLink("acceptable_ads_criteria", function(link)
63 { 67 {
64 setLinks("acceptableAdsExplanation", link, openFilters); 68 setLinks("acceptableAdsExplanation", link, openFilters);
65 }); 69 });
66 70
67 getDocLink("contribute", function(link) 71 getDocLink("contribute", function(link)
68 { 72 {
69 setLinks("share-headline", link); 73 setLinks("share-headline", link);
70 }); 74 });
71 75
72 ext.backgroundPage.sendMessage({type: "app.issues"}, function(issues) 76 ext.backgroundPage.sendMessage({
77 type: "app.get",
78 what: "issues"
79 }, function(issues)
73 { 80 {
74 // Show warning if data corruption was detected 81 // Show warning if data corruption was detected
75 if (issues.seenDataCorruption) 82 if (issues.seenDataCorruption)
76 { 83 {
77 E("dataCorruptionWarning").removeAttribute("hidden"); 84 E("dataCorruptionWarning").removeAttribute("hidden");
78 getDocLink("knownIssuesChrome_filterstorage", function(link) 85 getDocLink("knownIssuesChrome_filterstorage", function(link)
79 { 86 {
80 setLinks("dataCorruptionWarning", link); 87 setLinks("dataCorruptionWarning", link);
81 }); 88 });
82 } 89 }
83 90
84 // Show warning if filterlists settings were reinitialized 91 // Show warning if filterlists settings were reinitialized
85 if (issues.filterlistsReinitialized) 92 if (issues.filterlistsReinitialized)
86 { 93 {
87 E("filterlistsReinitializedWarning").removeAttribute("hidden"); 94 E("filterlistsReinitializedWarning").removeAttribute("hidden");
88 setLinks("filterlistsReinitializedWarning", openFilters); 95 setLinks("filterlistsReinitializedWarning", openFilters);
89 } 96 }
90 }); 97
91 98 if (issues.legacySafariVersion)
92 // Show warning if Safari version isn't supported
93 ext.backgroundPage.sendMessage({type: "app.info"}, function(info)
94 {
95 if (info.platform == "safari" && (
96 parseInt(info.platformVersion, 10) < 6 || // beforeload breaks websites in Safari 5
97 info.platformVersion == "6.1" || // extensions are broken in 6 .1 and 7.0
Sebastian Noack 2014/12/16 14:35:07 The old code used Services.vc.compare() here to ma
Wladimir Palant 2014/12/17 13:13:37 That's way more message passing than this is worth
Sebastian Noack 2014/12/17 13:45:22 Not necessarily. You could just add a new message
Wladimir Palant 2014/12/17 14:37:04 Frankly, neither approach sounds like worth doing
Sebastian Noack 2014/12/17 15:08:55 Well, the former approach wouldn't add any complex
Thomas Greiner 2014/12/18 10:17:48 I could think of a nice middleway. The current imp
Wladimir Palant 2014/12/18 19:31:35 Changed that, as discussed with Thomas: app.info m
98 info.platformVersion == "7.0"
99 ))
100 {
101 E("legacySafariWarning").removeAttribute("hidden"); 99 E("legacySafariWarning").removeAttribute("hidden");
102 }
103 }); 100 });
104 101
105 // Set up feature buttons linked to subscriptions 102 // Set up feature buttons linked to subscriptions
106 featureSubscriptions.forEach(initToggleSubscriptionButton); 103 featureSubscriptions.forEach(initToggleSubscriptionButton);
107 updateToggleButtons(); 104 updateToggleButtons();
108 initSocialLinks(); 105 updateSocialLinks();
109 106
110 ext.onMessage.addListener(function(message) 107 ext.onMessage.addListener(function(message)
111 { 108 {
112 if (message.type == "subscriptions.listen") 109 if (message.type == "subscriptions.listen")
113 { 110 {
114 updateToggleButtons(); 111 updateToggleButtons();
115 initSocialLinks(); 112 updateSocialLinks();
116 } 113 }
117 }); 114 });
118 ext.backgroundPage.sendMessage({type: "subscriptions.listen", filter: ["adde d", "removed", "disabled"]}); 115 ext.backgroundPage.sendMessage({
Thomas Greiner 2014/12/18 10:17:48 Nit: This line doesn't need to be that long.
Thomas Greiner 2014/12/18 10:17:48 By renaming "filter" to "args" it would become con
Wladimir Palant 2014/12/18 19:31:35 It would also become inconsistent with my objectio
116 type: "subscriptions.listen",
117 filter: ["added", "removed", "disabled"]
118 });
119 } 119 }
120 120
121 function initToggleSubscriptionButton(featureSubscription) 121 function initToggleSubscriptionButton(featureSubscription)
122 { 122 {
123 var feature = featureSubscription.feature; 123 var feature = featureSubscription.feature;
124 124
125 var element = E("toggle-" + feature); 125 var element = E("toggle-" + feature);
126 element.addEventListener("click", function(event) 126 element.addEventListener("click", function(event)
127 { 127 {
128 ext.backgroundPage.sendMessage({ 128 ext.backgroundPage.sendMessage({
129 type: "subscriptions.toggle", 129 type: "subscriptions.toggle",
130 url: featureSubscription.url, 130 url: featureSubscription.url,
131 title: featureSubscription.title, 131 title: featureSubscription.title,
132 homepage: featureSubscription.homepage 132 homepage: featureSubscription.homepage
133 }); 133 });
Thomas Greiner 2014/12/18 10:17:48 I wouldn't put the reserved "type" property in the
Wladimir Palant 2014/12/18 19:31:35 Fine with me if we have a new API that will accept
Thomas Greiner 2014/12/19 10:53:38 Sounds like a good compromise to me.
134 }, false); 134 }, false);
135 } 135 }
136 136
137 function openSharePopup(url) 137 function openSharePopup(url)
138 { 138 {
139 var iframe = E("share-popup"); 139 var iframe = E("share-popup");
140 var glassPane = E("glass-pane"); 140 var glassPane = E("glass-pane");
141 var popupMessageReceived = false; 141 var popupMessageReceived = false;
142 142
143 var popupMessageListener = function(event) 143 var popupMessageListener = function(event)
144 { 144 {
145 if (!/[.\/]adblockplus\.org$/.test(event.origin)) 145 if (!/[.\/]adblockplus\.org$/.test(event.origin))
Sebastian Noack 2014/12/16 14:35:07 This would match http://example.com/www.adblockplu
Wladimir Palant 2014/12/16 15:17:18 event.origin isn't a URL - see https://developer.m
Sebastian Noack 2014/12/16 15:25:33 Got ya. Feel free to ignore this comment then.
146 return; 146 return;
147 147
148 var width = event.data.width; 148 var width = event.data.width;
149 var height = event.data.height; 149 var height = event.data.height;
150 iframe.width = width; 150 iframe.width = width;
151 iframe.height = height; 151 iframe.height = height;
152 iframe.style.marginTop = -height/2 + "px"; 152 iframe.style.marginTop = -height/2 + "px";
153 iframe.style.marginLeft = -width/2 + "px"; 153 iframe.style.marginLeft = -width/2 + "px";
154 popupMessageReceived = true; 154 popupMessageReceived = true;
155 window.removeEventListener("message", popupMessageListener); 155 window.removeEventListener("message", popupMessageListener);
(...skipping 21 matching lines...) Expand all
177 } 177 }
178 178
179 iframe.removeEventListener("load", popupLoadListener); 179 iframe.removeEventListener("load", popupLoadListener);
180 }; 180 };
181 iframe.addEventListener("load", popupLoadListener, false); 181 iframe.addEventListener("load", popupLoadListener, false);
182 182
183 iframe.src = url; 183 iframe.src = url;
184 glassPane.className = "visible"; 184 glassPane.className = "visible";
185 } 185 }
186 186
187 function initSocialLinks() 187 function updateSocialLinks()
188 { 188 {
189 var networks = ["twitter", "facebook", "gplus"]; 189 var networks = ["twitter", "facebook", "gplus"];
190 networks.forEach(function(network) 190 networks.forEach(function(network)
191 { 191 {
192 var link = E("share-" + network); 192 var link = E("share-" + network);
193 var message = { 193 var message = {
Thomas Greiner 2014/12/18 10:17:48 Same as above regarding "type" property collisions
194 type: "filters.blocked", 194 type: "filters.blocked",
195 url: link.getAttribute("data-script"), 195 url: link.getAttribute("data-script"),
196 requestType: "SCRIPT", 196 requestType: "SCRIPT",
197 docDomain: "adblockplus.org", 197 docDomain: "adblockplus.org",
198 thirdParty: true 198 thirdParty: true
199 }; 199 };
200 ext.backgroundPage.sendMessage(message, function(blocked) 200 ext.backgroundPage.sendMessage(message, function(blocked)
201 { 201 {
202 // Don't open the share page if the sharing script would be blocked 202 // Don't open the share page if the sharing script would be blocked
203 if (blocked) 203 if (blocked)
204 link.removeEventListener("click", onSocialLinkClick, false); 204 link.removeEventListener("click", onSocialLinkClick, false);
Thomas Greiner 2014/12/18 10:17:48 No need to remove the event listener because it wo
Wladimir Palant 2014/12/18 19:31:35 This function is being called multiple times, when
Thomas Greiner 2014/12/19 10:53:38 Ok and thanks for adapting the function name to re
205 else 205 else
206 link.addEventListener("click", onSocialLinkClick, false); 206 link.addEventListener("click", onSocialLinkClick, false);
207 }); 207 });
208 }); 208 });
209 } 209 }
210 210
211 function onSocialLinkClick(event) 211 function onSocialLinkClick(event)
212 { 212 {
213 event.preventDefault(); 213 event.preventDefault();
214 214
(...skipping 23 matching lines...) Expand all
238 else if (typeof arguments[i + 1] == "function") 238 else if (typeof arguments[i + 1] == "function")
239 { 239 {
240 links[i].href = "javascript:void(0);"; 240 links[i].href = "javascript:void(0);";
241 links[i].addEventListener("click", arguments[i + 1], false); 241 links[i].addEventListener("click", arguments[i + 1], false);
242 } 242 }
243 } 243 }
244 } 244 }
245 245
246 function openFilters() 246 function openFilters()
247 { 247 {
248 ext.backgroundPage.sendMessage({type: "app.options"}); 248 ext.backgroundPage.sendMessage({type: "app.open", what: "options"});
249 } 249 }
250 250
251 function updateToggleButtons() 251 function updateToggleButtons()
252 { 252 {
253 ext.backgroundPage.sendMessage({type: "subscriptions.get"}, function(subscri ptions) 253 ext.backgroundPage.sendMessage({
Thomas Greiner 2014/12/18 10:17:48 What you want here is only downloadable subscripti
254 type: "subscriptions.get",
255 downloadable: true,
256 ignoreDisabled: true
257 }, function(subscriptions)
254 { 258 {
255 for (var i = 0; i < featureSubscriptions.length; i++) 259 for (var i = 0; i < featureSubscriptions.length; i++)
256 { 260 {
257 var featureSubscription = featureSubscriptions[i]; 261 var featureSubscription = featureSubscriptions[i];
258 updateToggleButton(featureSubscription.feature, subscriptions.indexOf(fe atureSubscription.url) >= 0); 262 updateToggleButton(featureSubscription.feature, subscriptions.indexOf(fe atureSubscription.url) >= 0);
259 } 263 }
260 }); 264 });
261 } 265 }
262 266
263 function updateToggleButton(feature, isEnabled) 267 function updateToggleButton(feature, isEnabled)
264 { 268 {
265 var button = E("toggle-" + feature); 269 var button = E("toggle-" + feature);
266 if (isEnabled) 270 if (isEnabled)
267 button.classList.remove("off"); 271 button.classList.remove("off");
268 else 272 else
269 button.classList.add("off"); 273 button.classList.add("off");
270 } 274 }
271 275
272 document.addEventListener("DOMContentLoaded", onDOMLoaded, false); 276 document.addEventListener("DOMContentLoaded", onDOMLoaded, false);
273 })(); 277 })();
LEFTRIGHT

Powered by Google App Engine
This is Rietveld