include.postload.js - Issue 5225119261655040: Issue 1282 - Don't generate filters conflicting with existing exception rules

Delta Between Two Patch Sets: include.postload.js

Issue 5225119261655040: Issue 1282 - Don't generate filters conflicting with existing exception rules (Closed)

Left Patch Set: Rebased Created Feb. 28, 2015, 6:03 p.m.

Right Patch Set: Addressed comment Created March 3, 2015, 2:59 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

LEFT	RIGHT
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

11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the	11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

12 * GNU General Public License for more details.	12 * GNU General Public License for more details.

13 *	13 *

14 * You should have received a copy of the GNU General Public License	14 * You should have received a copy of the GNU General Public License

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

16 */	16 */

17	17

18 // Click-to-hide stuff	18 // Click-to-hide stuff

19 var clickHide_activated = false;	19 var clickHide_activated = false;

20 var clickHide_filters = null;	20 var clickHide_filters = null;

21 var currentElement = null;	21 var currentElement = null;

22 var highlightedElementsSelector = null;	22 var highlightedElementsSelector = null;

23 var clickHideFiltersDialog = null;	23 var clickHideFiltersDialog = null;

24 var lastRightClickEvent = null;	24 var lastRightClickEvent = null;

25 var lastRightClickEventValid = false;	25 var lastRightClickEventValid = false;

	26 var lastMouseOverEvent = null;

26	27

27 function highlightElement(element, shadowColor, backgroundColor)	28 function highlightElement(element, shadowColor, backgroundColor)

28 {	29 {

29 unhighlightElement(element);	30 unhighlightElement(element);

30	31

31 var highlightWithOverlay = function()	32 var highlightWithOverlay = function()

32 {	33 {

33 var overlay = addElementOverlay(element);	34 var overlay = addElementOverlay(element);

34	35

35 // If the element isn't displayed no overlay will be added.	36 // If the element isn't displayed no overlay will be added.

(...skipping 346 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
382 }	383 }

383	384

384 function clickHide_elementClickHandler(e) {	385 function clickHide_elementClickHandler(e) {

385 e.preventDefault();	386 e.preventDefault();

386 e.stopPropagation();	387 e.stopPropagation();

387 clickHide_mouseClick(e);	388 clickHide_mouseClick(e);

388 }	389 }

389	390

390 function getBlockableElementOrAncestor(element, callback)	391 function getBlockableElementOrAncestor(element, callback)

391 {	392 {

	393 // We assume that the user doesn't want to block the whole page.

	394 // So we never consider the <html> or <body> element.

	395 while (element && element != document.documentElement

	396 && element != document.body)

	397 {

	398 // We can't handle non-HTML (like SVG) elements, as well as

	399 // <area> elements (see below). So fall back to the parent element.

	400 if (!(element instanceof HTMLElement) \|\| element.localName == "area")

	401 element = element.parentElement;

	402

	403 // If image maps are used mouse events occur for the <area> element.

	404 // But we have to block the image associated with the <map> element.

	405 else if (element.localName == "map")

	406 {

	407 var images = document.querySelectorAll("img[usemap]");

	408 var image = null;

	409

	410 for (var i = 0; i < images.length; i++)

	411 {

	412 var usemap = image.getAttribute("usemap");

	413 var index = usemap.indexOf("#");

	414

	415 if (index != -1 && usemap.substr(index + 1) == element.name)

	416 {

	417 image = images[i];

	418 break;

	419 }

	420 }

	421

	422 element = image;

	423 }

	424

	425 // Finally, if none of the above is true, check whether we can generate

	426 // any filters for this element. Otherwise fall back to its parent element.

	427 else

	428 {

	429 getFiltersForElement(element, function(filters)

	430 {

	431 if (filters.length > 0)

	432 callback(element);

	433 else

	434 getBlockableElementOrAncestor(element.parentElement, callback);

	435 });

	436

	437 return;

	438 }

	439 }

	440

392 // We reached the document root without finding a blockable element.	441 // We reached the document root without finding a blockable element.

393 // We also assume that the user doesn't want to block the whole page.	442 callback(null);

394 // So we never consider the <html> or <body> element.

395 if (!element \|\| element == document.documentElement \|\| element == document.bod y)

396 callback(null);

397

398 // We can't handle non-HTML (like SVG) elements, as well as

399 // <area> elements (see below). So fall back to the parent element.

400 else if (!(element instanceof HTMLElement) \|\| element.localName == "area")

401 getBlockableElementOrAncestor(element.parentElement, callback);
Wladimir Palant 2015/03/02 20:14:19 This seems to be the right recipe to error out wit This seems to be the right recipe to error out with a "too much recursion" error, e.g. when this function is applied to an element deep inside the SVG hierarchy. Chrome seems to allow really massive recursion (my test function crashed only after 17800 recursive calls) but that isn't something to be relied upon. Whenever we can simply iterate we should do that rather than making recursive calls. Sebastian Noack 2015/03/03 14:29:00 Done. Show quoted text On 2015/03/02 20:14:19, Wladimir Palant wrote: > This seems to be the right recipe to error out with a "too much recursion" > error, e.g. when this function is applied to an element deep inside the SVG > hierarchy. Chrome seems to allow really massive recursion (my test function > crashed only after 17800 recursive calls) but that isn't something to be relied > upon. Whenever we can simply iterate we should do that rather than making > recursive calls. Done.
402

403 // If image maps are used mouse events occur for the <area> element.

404 // But we have to block the image associated with the <map> element.

405 else if (element.localName == "map")

406 {

407 var images = document.querySelectorAll("img[usemap]");

408 var image = null;

409

410 for (var i = 0; i < images.length; i++)

411 {

412 var usemap = images[i].getAttribute("usemap");

413 var index = usemap.indexOf("#");

414

415 if (index != -1 && usemap.substr(index + 1) == element.name)

416 {

417 image = images[i];

418 break;

419 }

420 }

421

422 getBlockableElementOrAncestor(image, callback);

423 }

424

425 // Finally, if none of the above is true, check whether we can generate

426 // any filters for this element. Otherwise fall back to its parent element.

427 else

428 {

429 getFiltersForElement(element, function(filters)

430 {

431 if (filters.length > 0)

432 callback(element);

433 else

434 getBlockableElementOrAncestor(element.parentElement, callback);

435 });

436 }

437 }	443 }

438	444

439 // Hovering over an element so highlight it	445 // Hovering over an element so highlight it

440 function clickHide_mouseOver(e)	446 function clickHide_mouseOver(e)

441 {	447 {

442 if (clickHide_activated == false)	448 lastMouseOverEvent = e;

443 return;

444	449

445 getBlockableElementOrAncestor(e.target, function(element)	450 getBlockableElementOrAncestor(e.target, function(element)

446 {	451 {

447 if (currentElement)	452 if (e == lastMouseOverEvent)

448 unhighlightElement(currentElement);	453 {
Wladimir Palant 2015/03/02 20:14:19 I don't think it is that simple. Mouse events can I don't think it is that simple. Mouse events can be generated very quickly, sometimes even synchronously (e.g. if the handler on the webpage changes layout in such a way that another element is moved under the mouse pointer). So by the time you get a response from the background page it might be outdated already. One solution I see here is making only the most recent request count: a global variable is used as counter for mouseOver requests. Before calling getBlockableElementOrAncestor() this function stores a copy of that counter - after receiving a response it compares the global variable with its own copy. If they don't match the response is discarded, it means that a newer request has been submitted already. Sebastian Noack 2015/03/03 14:29:00 I have a hard time verifying that the suggested lo Show quoted text On 2015/03/02 20:14:19, Wladimir Palant wrote: > I don't think it is that simple. Mouse events can be generated very quickly, > sometimes even synchronously (e.g. if the handler on the webpage changes layout > in such a way that another element is moved under the mouse pointer). So by the > time you get a response from the background page it might be outdated already. > > One solution I see here is making only the most recent request count: a global > variable is used as counter for mouseOver requests. Before calling > getBlockableElementOrAncestor() this function stores a copy of that counter - > after receiving a response it compares the global variable with its own copy. If > they don't match the response is discarded, it means that a newer request has > been submitted already. I have a hard time verifying that the suggested logic with the counter is correct. How about just recording the last mouse event, bailing out in the callback if it changed?
449	454 lastMouseOverEvent = null;

450 if (element)	455

451 {	456 if (clickHide_activated)

452 highlightElement(element, "#d6d84b", "#f8fa47");	457 {

453 element.addEventListener("contextmenu", clickHide_elementClickHandler, tru e);	458 if (currentElement)

454 }	459 unhighlightElement(currentElement);

455	460

456 currentElement = element;	461 if (element)

	462 {

	463 highlightElement(element, "#d6d84b", "#f8fa47");

	464 element.addEventListener("contextmenu", clickHide_elementClickHandler, true);

	465 }

	466

	467 currentElement = element;

	468 }

	469 }

457 });	470 });

458	471

459 e.stopPropagation();	472 e.stopPropagation();

460 }	473 }

461	474

462 // No longer hovering over this element so unhighlight it	475 // No longer hovering over this element so unhighlight it

463 function clickHide_mouseOut(e)	476 function clickHide_mouseOut(e)

464 {	477 {

465 if (!clickHide_activated \|\| currentElement != e.target)	478 if (!clickHide_activated \|\| currentElement != e.target)
kzar 2015/03/02 18:30:08 Wouldn't e.target not match for image maps where t Wouldn't e.target not match for image maps where the element we're blocking is the image but the mouse events register for the area elements as explained above? Sebastian Noack 2015/03/02 18:40:48 currentElement is the element we are highlighting Show quoted text On 2015/03/02 18:30:08, kzar wrote: > Wouldn't e.target not match for image maps where the element we're blocking is > the image but the mouse events register for the area elements as explained > above? currentElement is the element we are highlighting (the image in that case), and the this code unhighlights it, when moving the mouse out of that element. I don't see any issues here. kzar 2015/03/02 18:42:29 Whoops missed the return there, OK. Show quoted text On 2015/03/02 18:40:48, Sebastian Noack wrote: > On 2015/03/02 18:30:08, kzar wrote: > > Wouldn't e.target not match for image maps where the element we're blocking is > > the image but the mouse events register for the area elements as explained > > above? > > currentElement is the element we are highlighting (the image in that case), and > the this code unhighlights it, when moving the mouse out of that element. I > don't see any issues here. Whoops missed the return there, OK.
466 return;	479 return;

467	480

468 unhighlightElement(currentElement);	481 unhighlightElement(currentElement);

469 currentElement.removeEventListener("contextmenu", clickHide_elementClickHandle r, true);	482 currentElement.removeEventListener("contextmenu", clickHide_elementClickHandle r, true);

470 e.stopPropagation();	483 e.stopPropagation();

471 }	484 }

472	485

473 // Selects the currently hovered-over filter or cancels selection	486 // Selects the currently hovered-over filter or cancels selection

474 function clickHide_keyDown(e)	487 function clickHide_keyDown(e)

475 {	488 {

(...skipping 250 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
726 lastRightClickEventValid = false;	739 lastRightClickEventValid = false;

727 else	740 else

728 lastRightClickEvent = null;	741 lastRightClickEvent = null;

729 break;	742 break;

730 }	743 }

731 });	744 });

732	745

733 if (window == window.top)	746 if (window == window.top)

734 ext.backgroundPage.sendMessage({type: "report-html-page"});	747 ext.backgroundPage.sendMessage({type: "report-html-page"});

735 }	748 }

LEFT	RIGHT