options.js - Issue 29321417: Issue 2357 - Added "predefined list" dialog to options page

Side by Side Diff: options.js

Issue 29321417: Issue 2357 - Added "predefined list" dialog to options page (Closed)

Patch Set: Created July 3, 2015, 3:55 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 /*	1 /*

2 * This file is part of Adblock Plus <https://adblockplus.org/>,	2 * This file is part of Adblock Plus <https://adblockplus.org/>,

3 * Copyright (C) 2006-2015 Eyeo GmbH	3 * Copyright (C) 2006-2015 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 298 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
309 if (element.getAttribute("popular"))	309 if (element.getAttribute("popular"))

310 recommendation.isPopular = true;	310 recommendation.isPopular = true;

311	311

312 recommendationsMap[subscription.url] = recommendation;	312 recommendationsMap[subscription.url] = recommendation;

313 updateSubscription(subscription);	313 updateSubscription(subscription);

314 }	314 }

315 }, false);	315 }, false);

316 request.send(null);	316 request.send(null);

317 }	317 }

318	318

	319 function onClick(e)

	320 {

	321 var element = e.target;

	322 while (true)

	323 {

	324 if (element === document.body)
	Sebastian Noack 2015/07/09 12:24:18 What if the click event occurs on the <html> eleme What if the click event occurs on the <html> element? Probably the simplest and most robust solution would be to simply check for element not being null here, and using element.parentElement (which will be null instead document when you reach the root) below. Sebastian Noack 2015/07/09 12:24:19 Nit: We don't use the strict equality operator unl Nit: We don't use the strict equality operator unless necessary. This is also in the Mozilla coding guide, which our coding guide refers to. Thomas Greiner 2015/07/09 13:55:06 I can't find that section in the Mozilla coding st Show quoted text On 2015/07/09 12:24:19, Sebastian Noack wrote: > Nit: We don't use the strict equality operator unless necessary. This is also in > the Mozilla coding guide, which our coding guide refers to. I can't find that section in the Mozilla coding style, it only says "In JavaScript, === is preferred to ==." (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style...) Thomas Greiner 2015/07/09 13:55:07 Done. Note that it doesn't make a difference in th Show quoted text On 2015/07/09 12:24:18, Sebastian Noack wrote: > What if the click event occurs on the <html> element? > > Probably the simplest and most robust solution would be to simply check for > element not being null here, and using element.parentElement (which will be null > instead document when you reach the root) below. Done. Note that it doesn't make a difference in this scenario because we're only interested in events that happen within the page body inside an element that has the "data-action" attribute set. Sebastian Noack 2015/07/09 15:16:56 Yes, we are, however if the "click" is is triggere Show quoted text On 2015/07/09 13:55:07, Thomas Greiner wrote: > On 2015/07/09 12:24:18, Sebastian Noack wrote: > > What if the click event occurs on the <html> element? > > > > Probably the simplest and most robust solution would be to simply check for > > element not being null here, and using element.parentElement (which will be > null > > instead document when you reach the root) below. > > Done. > Note that it doesn't make a difference in this scenario because we're only > interested in events that happen within the page body inside an element that has > the "data-action" attribute set. Yes, we are, however if the "click" is is triggered for the document element, you would never bail out, if you check only for document.body. Sebastian Noack 2015/07/09 15:16:57 You are right, I thought it were the other way aro Show quoted text On 2015/07/09 13:55:06, Thomas Greiner wrote: > On 2015/07/09 12:24:19, Sebastian Noack wrote: > > Nit: We don't use the strict equality operator unless necessary. This is also > in > > the Mozilla coding guide, which our coding guide refers to. > > I can't find that section in the Mozilla coding style, it only says "In > JavaScript, === is preferred to ==." > (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style...) You are right, I thought it were the other way around. But anyway, it seems that we use == and != (instead === and !==) in most of our code. And frankly I, and apparently some others, prefer it this way as it keeps the code concise, and IMO also more simple.
	325 return;

	326

	327 if ("action" in element.dataset)

	328 break;

	329

	330 element = element.parentNode;
	Sebastian Noack 2015/07/09 15:16:56 This should be \|element.parentElement\| as mentione This should be \|element.parentElement\| as mentioned before. Thomas Greiner 2015/07/10 14:19:15 Done. Show quoted text On 2015/07/09 15:16:56, Sebastian Noack wrote: > This should be \|element.parentElement\| as mentioned before. Done.
	331 }

	332

	333 switch (element.dataset.action)
	Sebastian Noack 2015/07/09 12:24:18 Older Safari versions don't have dataset. Older Safari versions don't have dataset. Thomas Greiner 2015/07/09 13:55:06 According to caniuse.com it has been supported sin Show quoted text On 2015/07/09 12:24:18, Sebastian Noack wrote: > Older Safari versions don't have dataset. According to caniuse.com it has been supported since version 5.1 (http://caniuse.com/#search=dataset) while MDN says it was introduced in Safari 6 (https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset#Browser_...). AFAIK we no longer support versions older than Safari 6 so that should be sufficient for us, right? Otherwise I can simply modify the code to use "getAttribute" and "setAttribute". Sebastian Noack 2015/07/09 15:16:56 Officially, we support Safari 6+. That is mostly b Show quoted text On 2015/07/09 13:55:06, Thomas Greiner wrote: > On 2015/07/09 12:24:18, Sebastian Noack wrote: > > Older Safari versions don't have dataset. > > According to http://caniuse.com it has been supported since version 5.1 > (http://caniuse.com/#search=dataset) while MDN says it was introduced in Safari > 6 > (https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset#Browser_...). > AFAIK we no longer support versions older than Safari 6 so that should be > sufficient for us, right? > > Otherwise I can simply modify the code to use "getAttribute" and "setAttribute". Officially, we support Safari 6+. That is mostly because a WebKit bug causing side effects when registering any onbeforeload listener on older versions. Except for that issue, and slower performance, Adblock Plus still works on Safari 5.1. And is actually a fairly popular there, since everybody on OS X Snow Leopard as well as people using Safari on Windows are stuck with Safari 5.1. Last time we checked, more than 10% of our Safari users were on Safari 5.1. I'm fine with incomplete support of Safari 5.1. But breaking the options page when we could easily avoid it, seems unnecessary. saroyanm 2015/07/09 17:14:19 I will suggest to make that kind of modification i Show quoted text On 2015/07/09 15:16:56, Sebastian Noack wrote: > On 2015/07/09 13:55:06, Thomas Greiner wrote: > > On 2015/07/09 12:24:18, Sebastian Noack wrote: > > > Older Safari versions don't have dataset. > > > > According to http://caniuse.com it has been supported since version 5.1 > > (http://caniuse.com/#search=dataset) while MDN says it was introduced in > Safari > > 6 > > > (https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset#Browser_...). > > AFAIK we no longer support versions older than Safari 6 so that should be > > sufficient for us, right? > > > > Otherwise I can simply modify the code to use "getAttribute" and > "setAttribute". > > Officially, we support Safari 6+. That is mostly because a WebKit bug causing > side effects when registering any onbeforeload listener on older versions. > Except for that issue, and slower performance, Adblock Plus still works on > Safari 5.1. And is actually a fairly popular there, since everybody on OS X Snow > Leopard as well as people using Safari on Windows are stuck with Safari 5.1. > Last time we checked, more than 10% of our Safari users were on Safari 5.1. I'm > fine with incomplete support of Safari 5.1. But breaking the options page when > we could easily avoid it, seems unnecessary. I will suggest to make that kind of modification in the follow up ticket, I just found out that we don't have one, but I do remember that we wanted to have separate Stylesheet for older Safari versions, so I think we will need to test our new options page on safari 5.1 anyway. While initially we were designing new option page to support safari 6 and newer I would suggest to leave this for now, I do believe that It's not only modification that we will need to make and not sure how big will be our user base on safari 5.1 when we finish the new option page implementation. What you think ? Sebastian Noack 2015/07/10 07:58:19 Well, by relying on flexbox, the new option page a Well, by relying on flexbox, the new option page actually requires Safari 6.1+. IIRC, the plan was to have a minimal stylesheet for Safari 6.0, to support the most important functionality there, while hiding all other elements, that it doesn't look broken. Safari 5.1 has way more users than Safari 6.0. But if we support Safari 6.0, supporting Safari 5.1 with the options page as well would be as trivial as using setAttribute/getAttriubte instead dataset. Also note that we would need to do that as well to support IE 8-10. Thomas Greiner 2015/07/10 14:19:15 Done. I wouldn't consider IE support to be releva Show quoted text On 2015/07/10 07:58:19, Sebastian Noack wrote: > Well, by relying on flexbox, the new option page actually requires Safari 6.1+. > IIRC, the plan was to have a minimal stylesheet for Safari 6.0, to support the > most important functionality there, while hiding all other elements, that it > doesn't look broken. > > Safari 5.1 has way more users than Safari 6.0. But if we support Safari 6.0, > supporting Safari 5.1 with the options page as well would be as trivial as using > setAttribute/getAttriubte instead dataset. Also note that we would need to do > that as well to support IE 8-10. Done. I wouldn't consider IE support to be relevant at this point of development because of the general overhead that this would introduce.
	334 {

	335 case "add-domain-exception":

	336 addWhitelistedDomain();

	337 break;

	338 case "add-subscription":

	339 var dialog = E("dialog-content-addSubscription");

	340 var title = dialog.querySelector("h3").textContent;

	341 var url = dialog.querySelector(".url").textContent;

	342 addEnableSubscription(url, title);

	343 delete document.body.dataset.dialog;

	344 break;

	345 case "cancel-domain-exception":

	346 E("whitelisting-textbox").value = "";

	347 break;

	348 case "close-dialog":

	349 delete document.body.dataset.dialog;

	350 break;

	351 case "edit-custom-filters":

	352 editCustomFilters();

	353 break;

	354 case "import-subscription":

	355 var url = E("blockingList-textbox").value;

	356 addEnableSubscription(url);

	357 delete document.body.dataset.dialog;

	358 break;

	359 case "open-language-dialog":

	360 openDialog("language");

	361 break;

	362 case "open-subscription-dialog":

	363 openDialog("customlist");

	364 break;

	365 }

	366 }

	367

319 function onDOMLoaded()	368 function onDOMLoaded()

320 {	369 {

321 var recommendationTemplate = document.querySelector("#recommend-list-table t emplate");	370 var recommendationTemplate = document.querySelector("#recommend-list-table t emplate");

322 var popularText = ext.i18n.getMessage("options_popular");	371 var popularText = ext.i18n.getMessage("options_popular");

323 recommendationTemplate.content.querySelector(".popular").textContent = popul arText;	372 recommendationTemplate.content.querySelector(".popular").textContent = popul arText;

324 var languagesTemplate = document.querySelector("#all-lang-table template");	373 var languagesTemplate = document.querySelector("#all-lang-table template");

325 var buttonText = ext.i18n.getMessage("options_button_add");	374 var buttonText = ext.i18n.getMessage("options_button_add");

326 languagesTemplate.content.querySelector(".button-add span").textContent = bu ttonText;	375 languagesTemplate.content.querySelector(".button-add span").textContent = bu ttonText;

327	376

328 populateLists();	377 populateLists();

(...skipping 32 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
361 });	410 });

362	411

363 getDocLink("contribute", function(link)	412 getDocLink("contribute", function(link)

364 {	413 {

365 document.querySelector("#tab-contribute a").setAttribute("href", link);	414 document.querySelector("#tab-contribute a").setAttribute("href", link);

366 });	415 });

367	416

368 updateShareLink();	417 updateShareLink();

369	418

370 // Initialize interactive UI elements	419 // Initialize interactive UI elements

	420 document.body.addEventListener("click", onClick, false);

371 var placeholderValue = ext.i18n.getMessage("options_dialog_language_find");	421 var placeholderValue = ext.i18n.getMessage("options_dialog_language_find");

372 E("find-language").setAttribute("placeholder", placeholderValue);	422 E("find-language").setAttribute("placeholder", placeholderValue);

373 E("add-blocking-list").addEventListener("click", function()

374 {

375 openDialog("customlist");

376 }, false);

377 E("add-website-language").addEventListener("click", function()

378 {

379 openDialog("language");

380 }, false);

381 E("dialog-close").addEventListener("click", function()

382 {

383 delete document.body.dataset.dialog;

384 }, false);

385 E("edit-ownBlockingList-button").addEventListener("click", editCustomFilters , false);

386 E("find-language").addEventListener("keyup", onFindLanguageKeyUp, false);	423 E("find-language").addEventListener("keyup", onFindLanguageKeyUp, false);

387 E("whitelisting").addEventListener("click", function(e)

388 {

389 var id = e.target.id;

390 if (id == "whitelisting-add-icon" \|\| id == "whitelisting-enter-icon")

391 addWhitelistedDomain();

392 else if (id == "whitelisting-cancel-button")

393 E("whitelisting-textbox").value = "";

394 }, false);

395 E("whitelisting-add-button").addEventListener("click", addWhitelistedDomain, false);

396 E("whitelisting-textbox").addEventListener("keypress", function(e)	424 E("whitelisting-textbox").addEventListener("keypress", function(e)

397 {	425 {

398 // e.keyCode has been deprecated so we attempt to use e.key	426 // e.keyCode has been deprecated so we attempt to use e.key

399 // keyCode "13" corresponds to "Enter"	427 // keyCode "13" corresponds to "Enter"

400 if ((e.key && e.key == "Enter") \|\| (!e.key && e.keyCode == 13))	428 if ((e.key && e.key == "Enter") \|\| (!e.key && e.keyCode == 13))

401 addWhitelistedDomain();	429 addWhitelistedDomain();

402 }, false);	430 }, false);

403 E("import-blockingList-button").addEventListener("click", function()

404 {

405 var url = E("blockingList-textbox").value;

406 addEnableSubscription(url);

407 delete document.body.dataset.dialog;

408 }, false);

409 }	431 }

410	432

411 function openDialog(name)	433 function openDialog(name)

412 {	434 {

413 document.body.dataset.dialog = name;	435 document.body.dataset.dialog = name;

414 }	436 }

415	437

416 function populateLists()	438 function populateLists()

417 {	439 {

418 subscriptionsMap = Object.create(null);	440 subscriptionsMap = Object.create(null);

(...skipping 180 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
599 });	621 });

600 break;	622 break;

601 case "title":	623 case "title":

602 // TODO: NYI	624 // TODO: NYI

603 break;	625 break;

604 }	626 }

605 }	627 }

606	628

607 function showAddSubscriptionDialog(subscription)	629 function showAddSubscriptionDialog(subscription)

608 {	630 {

609 E("blockingList-textbox").value = subscription.url;	631 var dialog = E("dialog-content-addSubscription");

610 openDialog("customlist");	632 dialog.querySelector("h3").textContent = subscription.title \|\| "";

	633 dialog.querySelector(".url").textContent = subscription.url;

	634 openDialog("addSubscription");

611 }	635 }

612	636

613 function updateShareLink()	637 function updateShareLink()

614 {	638 {

615 ext.backgroundPage.sendMessage(	639 ext.backgroundPage.sendMessage(

616 {	640 {

617 type: "filters.blocked",	641 type: "filters.blocked",

618 url: "https://platform.twitter.com/widgets/",	642 url: "https://platform.twitter.com/widgets/",

619 requestType: "SCRIPT",	643 requestType: "SCRIPT",

620 docDomain: "adblockplus.org",	644 docDomain: "adblockplus.org",

(...skipping 48 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
669 filter: ["added", "loaded", "removed"]	693 filter: ["added", "loaded", "removed"]

670 });	694 });

671 ext.backgroundPage.sendMessage(	695 ext.backgroundPage.sendMessage(

672 {	696 {

673 type: "subscriptions.listen",	697 type: "subscriptions.listen",

674 filter: ["added", "disabled", "homepage", "removed", "title"]	698 filter: ["added", "disabled", "homepage", "removed", "title"]

675 });	699 });

676	700

677 window.addEventListener("DOMContentLoaded", onDOMLoaded, false);	701 window.addEventListener("DOMContentLoaded", onDOMLoaded, false);

678 })();	702 })();

OLD	NEW

« no previous file with comments | « options.html ('k') | skin/options.css » ('j') | no next file with comments »