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

Delta Between Two Patch Sets: background.js

Issue 16067002: Added Safari Support (Closed)
Left Patch Set: Addressed comments Created Nov. 12, 2013, 10:28 a.m.
Right Patch Set: Bugfixes Created Nov. 15, 2013, 8:58 a.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 | « no previous file | block.js » ('j') | 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-2013 Eyeo GmbH 3 * Copyright (C) 2006-2013 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 96 matching lines...) Expand 10 before | Expand all | Expand 10 after
107 var result = defaultMatcher.matchesAny(url, type || "DOCUMENT", extractHostFro mURL(parentUrl || url), false); 107 var result = defaultMatcher.matchesAny(url, type || "DOCUMENT", extractHostFro mURL(parentUrl || url), false);
108 return (result instanceof WhitelistFilter ? result : null); 108 return (result instanceof WhitelistFilter ? result : null);
109 } 109 }
110 110
111 var activeNotification = null; 111 var activeNotification = null;
112 112
113 // Adds or removes page action icon according to options. 113 // Adds or removes page action icon according to options.
114 function refreshIconAndContextMenu(tab) 114 function refreshIconAndContextMenu(tab)
115 { 115 {
116 if(!/^https?:/.test(tab.url)) 116 if(!/^https?:/.test(tab.url))
117 return; 117 return;
Wladimir Palant 2013/11/12 15:19:52 Won't this make the icon get stuck in its previous
Sebastian Noack 2013/11/12 16:56:15 We can't control the pageAction on pages that aren
Wladimir Palant 2013/11/13 07:16:27 This isn't really true - we can show our icon on p
118 118
119 var iconFilename = "icons/abp-"; 119 var iconFilename;
120 if ("safari" in window) 120 if (require("info").platform == "safari")
Wladimir Palant 2013/11/12 15:19:52 Why resort to browser sniffing? You should use req
Sebastian Noack 2013/11/12 16:56:15 In this case I agree. But when checking for the av
Wladimir Palant 2013/11/13 07:16:27 I disagree. Please use require("info") for any bro
121 iconFilename += "16"; 121 // There is no grayscale version of the icon for whitelisted tabs
122 // when using Safari, because icons are grayscale already and icons
123 // aren't per tab in Safari.
124 iconFilename = "icons/abp-16.png"
122 else 125 else
123 { 126 {
124 iconFilename += "19";
125
126 // If the page is whitelisted use the grayscale version of the icon
127 // for that tab, except for Safari where all icons are grayscale
128 // already and where icons aren't per tab.
129 var excluded = isWhitelisted(tab.url); 127 var excluded = isWhitelisted(tab.url);
130 if (excluded) 128 iconFilename = excluded ? "icons/abp-19-whitelisted.png" : "icons/abp-19.png ";
131 iconFilename += "-whitelisted"; 129 }
Wladimir Palant 2013/11/12 15:19:52 The logic here is a lot more opaque than it was or
132 }
133 iconFilename += ".png";
134 130
135 tab.pageAction.setIcon(iconFilename); 131 tab.pageAction.setIcon(iconFilename);
136 tab.pageAction.setTitle("Adblock Plus"); 132 tab.pageAction.setTitle(ext.i18n.getMessage("name"));
Wladimir Palant 2013/11/12 15:19:52 Ouch, this should be localized - i18n.getMessage("
Sebastian Noack 2013/11/12 16:56:15 Yes, it should. However translation round already
Wladimir Palant 2013/11/13 07:16:27 There is - "name", imported from Firefox locales.
137 133
138 iconAnimation.registerTab(tab, iconFilename); 134 iconAnimation.registerTab(tab, iconFilename);
139 135
140 if (localStorage.shouldShowIcon == "false") 136 if (localStorage.shouldShowIcon == "false")
141 tab.pageAction.hide(); 137 tab.pageAction.hide();
142 else 138 else
143 tab.pageAction.show(); 139 tab.pageAction.show();
144 140
145 if ("chrome" in window) // TODO: Implement context menus for Safari 141 if (require("info").platform == "chromium") // TODO: Implement context menus f or Safari
146 // Set context menu status according to whether current tab has whitelisted domain 142 // Set context menu status according to whether current tab has whitelisted domain
147 if (excluded) 143 if (excluded)
148 chrome.contextMenus.removeAll(); 144 chrome.contextMenus.removeAll();
149 else 145 else
150 showContextMenu(); 146 showContextMenu();
151 } 147 }
152 148
153 /** 149 /**
154 * Old versions stored filter data in the localStorage object, this will import 150 * Old versions stored filter data in the localStorage object, this will import
155 * it into FilterStorage properly. 151 * it into FilterStorage properly.
(...skipping 212 matching lines...) Expand 10 before | Expand all | Expand 10 after
368 } 364 }
369 else 365 else
370 addAcceptable = false; 366 addAcceptable = false;
371 } 367 }
372 368
373 if (!addSubscription && !addAcceptable) 369 if (!addSubscription && !addAcceptable)
374 return; 370 return;
375 371
376 function notifyUser() 372 function notifyUser()
377 { 373 {
378 ext.windows.getLastFocused(function(win) { 374 ext.windows.getLastFocused(function(win)
Wladimir Palant 2013/11/12 15:19:52 Nit: that bracket should be on the next line.
375 {
379 win.openTab(ext.getURL("firstRun.html")); 376 win.openTab(ext.getURL("firstRun.html"));
380 }); 377 });
381 } 378 }
382 379
383 if (addSubscription) 380 if (addSubscription)
384 { 381 {
385 // Load subscriptions data 382 // Load subscriptions data
386 var request = new XMLHttpRequest(); 383 var request = new XMLHttpRequest();
387 request.open("GET", "subscriptions.xml"); 384 request.open("GET", "subscriptions.xml");
388 request.addEventListener("load", function() 385 request.addEventListener("load", function()
(...skipping 27 matching lines...) Expand all
416 { 413 {
417 chrome.contextMenus.create({"title": chrome.i18n.getMessage("block_element "), "contexts": ["image", "video", "audio"], "onclick": function(info, tab) 414 chrome.contextMenus.create({"title": chrome.i18n.getMessage("block_element "), "contexts": ["image", "video", "audio"], "onclick": function(info, tab)
418 { 415 {
419 if(info.srcUrl) 416 if(info.srcUrl)
420 chrome.tabs.sendRequest(tab.id, {reqtype: "clickhide-new-filter", fi lter: info.srcUrl}); 417 chrome.tabs.sendRequest(tab.id, {reqtype: "clickhide-new-filter", fi lter: info.srcUrl});
421 }}); 418 }});
422 } 419 }
423 }); 420 });
424 } 421 }
425 422
423 /**
424 * Opens options tab or focuses an existing one, within the last focused window .
425 * @param {Function} callback function to be called with the
426 Tab object of the options tab
427 */
426 function openOptions(callback) 428 function openOptions(callback)
427 { 429 {
428 ext.windows.getLastFocused(function(win) 430 ext.windows.getLastFocused(function(win)
Wladimir Palant 2013/11/12 15:19:52 Why restrict it to the current browser window? We
Sebastian Noack 2013/11/12 16:56:15 You are right about that the behavior changed here
Wladimir Palant 2013/11/13 07:16:27 No, the user experience is worse - users might uni
Sebastian Noack 2013/11/13 09:33:12 I disagree. When you click on the options link in
Felix Dahlke 2013/11/13 12:00:16 I'm with Wladimir here, what the browser does is w
Sebastian Noack 2013/11/13 12:26:34 What Wladimir suggested is not what the browser do
429 { 431 {
430 win.getAllTabs(function(tabs) 432 win.getAllTabs(function(tabs)
431 { 433 {
432 var optionsUrl = ext.getURL("options.html"); 434 var optionsUrl = ext.getURL("options.html");
433 435
434 for (var i = 0; i < tabs.length; i++) 436 for (var i = 0; i < tabs.length; i++)
Wladimir Palant 2013/11/12 15:19:52 Nit: please always put brackets around multi-line
437 {
435 if (tabs[i].url == optionsUrl) 438 if (tabs[i].url == optionsUrl)
436 { 439 {
437 tabs[i].activate(); 440 tabs[i].activate();
438 if (callback) 441 if (callback)
439 callback(tabs[i]); 442 callback(tabs[i]);
440 return; 443 return;
441 } 444 }
442 445 }
443 win.openTab(optionsUrl, callback && function(tab) { 446
Wladimir Palant 2013/11/12 15:19:52 Nit: this bracket should be on the next line.
447 win.openTab(optionsUrl, callback && function(tab)
448 {
444 tab.onCompleted.addListener(callback); 449 tab.onCompleted.addListener(callback);
445 }); 450 });
446 }); 451 });
447 }); 452 });
448 } 453 }
449 454
450 function prepareNotificationIconAndPopup() 455 function prepareNotificationIconAndPopup()
451 { 456 {
452 activeNotification.onClicked = function() 457 activeNotification.onClicked = function()
453 { 458 {
(...skipping 29 matching lines...) Expand all
483 for (var frameId in frames.get(tab)) 488 for (var frameId in frames.get(tab))
484 if (getFrameUrl(tab, frameId) == url) 489 if (getFrameUrl(tab, frameId) == url)
485 return frameId; 490 return frameId;
486 return -1; 491 return -1;
487 } 492 }
488 493
489 ext.onMessage.addListener(function (msg, sender, sendResponse) 494 ext.onMessage.addListener(function (msg, sender, sendResponse)
490 { 495 {
491 switch (msg.type) 496 switch (msg.type)
492 { 497 {
493 case "get-settings": 498 case "get-selectors":
494 var hostDomain = null;
495 var selectors = null; 499 var selectors = null;
496
497 var frameId = sender.tab ? getFrameId(sender.tab, msg.frameUrl) : -1; 500 var frameId = sender.tab ? getFrameId(sender.tab, msg.frameUrl) : -1;
498 var enabled = false; 501
499 502 if (!isFrameWhitelisted(sender.tab, frameId, "DOCUMENT") &&
500 if (!isFrameWhitelisted(sender.tab, frameId, "DOCUMENT")) 503 !isFrameWhitelisted(sender.tab, frameId, "ELEMHIDE"))
501 if (!isFrameWhitelisted(sender.tab, frameId, "ELEMHIDE")) { 504 {
Wladimir Palant 2013/11/12 15:19:52 As with other occasions, please use && here. In ge
502 var enabled = true; 505 var noStyleRules = false;
Wladimir Palant 2013/11/12 15:19:52 You are redeclaring the variable here. Side-note:
503 506 var host = extractHostFromURL(msg.frameUrl);
504 if (msg.selectors) 507 for (var i = 0; i < noStyleRulesHosts.length; i++)
505 { 508 {
506 var noStyleRules = false; 509 var noStyleHost = noStyleRulesHosts[i];
507 var host = extractHostFromURL(msg.frameUrl); 510 if (host == noStyleHost || (host.length > noStyleHost.length &&
508 hostDomain = getBaseDomain(host); 511 host.substr(host.length - noStyleHost.leng th - 1) == "." + noStyleHost))
509 for (var i = 0; i < noStyleRulesHosts.length; i++)
510 { 512 {
511 var noStyleHost = noStyleRulesHosts[i]; 513 noStyleRules = true;
512 if (host == noStyleHost || (host.length > noStyleHost.length &&
513 host.substr(host.length - noStyleHost.le ngth - 1) == "." + noStyleHost))
514 {
515 noStyleRules = true;
516 }
517 }
518 selectors = ElemHide.getSelectorsForDomain(host, false);
519 if (noStyleRules)
520 {
521 selectors = selectors.filter(function(s)
522 {
523 return !/\[style[\^\$]?=/.test(s);
524 });
525 } 514 }
526 } 515 }
527 } 516 selectors = ElemHide.getSelectorsForDomain(host, false);
528 517 if (noStyleRules)
529 sendResponse({enabled: enabled, hostDomain: hostDomain, selectors: selecto rs}); 518 {
519 selectors = selectors.filter(function(s)
520 {
521 return !/\[style[\^\$]?=/.test(s);
522 });
523 }
524 }
525
526 sendResponse(selectors);
530 break; 527 break;
531 case "should-collapse": 528 case "should-collapse":
532 var frameId = sender.tab ? getFrameId(sender.tab, msg.documentUrl) : -1; 529 var frameId = sender.tab ? getFrameId(sender.tab, msg.documentUrl) : -1;
533 530
534 if (isFrameWhitelisted(sender.tab, frameId, "DOCUMENT")) 531 if (isFrameWhitelisted(sender.tab, frameId, "DOCUMENT"))
535 { 532 {
536 sendResponse(false); 533 sendResponse(false);
537 break; 534 break;
538 } 535 }
539 536
(...skipping 21 matching lines...) Expand all
561 } 558 }
562 break; 559 break;
563 case "add-filters": 560 case "add-filters":
564 if (msg.filters && msg.filters.length) 561 if (msg.filters && msg.filters.length)
565 { 562 {
566 for (var i = 0; i < msg.filters.length; i++) 563 for (var i = 0; i < msg.filters.length; i++)
567 FilterStorage.addFilter(Filter.fromText(msg.filters[i])); 564 FilterStorage.addFilter(Filter.fromText(msg.filters[i]));
568 } 565 }
569 break; 566 break;
570 case "add-subscription": 567 case "add-subscription":
571 openOptions(function(tab) { tab.sendMessage(msg); }); 568 openOptions(function(tab)
Wladimir Palant 2013/11/12 15:19:52 Please don't compress this into one line - use the
569 {
570 tab.sendMessage(msg);
571 });
572 break; 572 break;
573 case "forward": 573 case "forward":
574 tab.sendMessage(msg.payload, sendResponse); 574 tab.sendMessage(msg.payload, sendResponse);
575 break; 575 break;
576 default: 576 default:
577 sendResponse({}); 577 sendResponse({});
578 break; 578 break;
579 } 579 }
580 }); 580 });
581 581
582 // Show icon as page action for all tabs that already exist 582 // Show icon as page action for all tabs that already exist
583 ext.windows.getAll(function(windows) 583 ext.windows.getAll(function(windows)
584 { 584 {
585 for (var i = 0; i < windows.length; i++) 585 for (var i = 0; i < windows.length; i++)
Wladimir Palant 2013/11/12 15:19:52 Nit: Please add brackets around the multi-line blo
586 {
586 windows[i].getAllTabs(function(tabs) 587 windows[i].getAllTabs(function(tabs)
587 { 588 {
588 tabs.forEach(refreshIconAndContextMenu); 589 tabs.forEach(refreshIconAndContextMenu);
589 }); 590 });
591 }
590 }); 592 });
591 593
592 // Update icon if a tab changes location 594 // Update icon if a tab changes location
593 ext.tabs.onBeforeNavigate.addListener(function(tab) 595 ext.tabs.onBeforeNavigate.addListener(function(tab)
594 { 596 {
595 tab.sendMessage({type: "clickhide-deactivate"}); 597 tab.sendMessage({type: "clickhide-deactivate"});
596 refreshIconAndContextMenu(tab); 598 refreshIconAndContextMenu(tab);
597 }); 599 });
598 600
599 setTimeout(function() 601 setTimeout(function()
600 { 602 {
601 var notificationToShow = Notification.getNextToShow(); 603 var notificationToShow = Notification.getNextToShow();
602 if (notificationToShow) 604 if (notificationToShow)
603 showNotification(notificationToShow); 605 showNotification(notificationToShow);
604 }, 3 * 60 * 1000); 606 }, 3 * 60 * 1000);
LEFTRIGHT
« no previous file | block.js » ('j') | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld