Opened on 02/26/2018 at 12:43:18 PM

Closed on 03/16/2019 at 03:45:55 PM

#6422 closed change (rejected)

Use user style sheets for emulation hiding filters if available

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Platform Keywords: circumvention
Cc: kzar, hfiguiere, sebastian, fhd, amrmak, arthur Blocked By: #6437, #6446, #6458, #6504
Blocking: #6459 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29714569/

Description (last modified by mjethani)

Background

Currently ABP uses user style sheets only for standard hiding filters, but there have been recent changes that enable us to use them for emulation hiding filters as well:

  1. Now we keep the style sheet for emulation hiding filters updated at all times so that there are no stale selectors (revision 8399e34fff05)
  2. If both tabs.insertCSS and tabs.removeCSS are available, we use user style sheets for some (but not all, i.e. only -abp-properties) emulation hiding filters (revision 7247eb4632fa)

With a little bit of work, we can start using user style sheets for -abp-has and -abp-contains as well.

What to change

In include.preload.js, set the useInlineStyles property on the ElemHideEmulation instance to false if user style sheets are fully supported, otherwise set it to true.

Resolution

See #6504. We no longer use style sheets for any element hiding emulation filters (not even :-abp-properties()).

Attachments (0)

Change History (19)

comment:1 follow-up: Changed on 02/26/2018 at 01:24:03 PM by mjethani

Actually we don't have to traverse the entire DOM every time there's a mutation just for -abp-has and -abp-contains.

Let's take this document:

  <div id="n1">
    <div id="n1_1"></div>
    <div id="n1_2"></div>
    <div id="n1_4">Hello</div>
  </div>
  <div id="n2">
    <div id="n2_1"></div>
    <div id="n2_2"></div>
    <div id="n2_4">Hello</div>
  </div>
  <div id="n3">
    <div id="n3_1"></div>
    <div id="n3_2"></div>
    <div id="n3_4">Hello</div>
  </div>
  <div id="n4">
    <div id="n4_1"></div>
    <div id="n4_2"></div>
    <div id="n4_4">Hello</div>
  </div>

And let's say we have the selector :-abp-contains(Hello)

Now suppose a new node n2_3 is added to the DOM. In that case, we only need to do the following:

  1. Run the filtering on n2_3, all its ancestors (n2 onwards), and its entire subtree
  2. For each node that was previously affected by the filtering (n1_4, n2_4, n3_4, and n4_4), calculate the path and update it if it has changed

In this case, the path of n2_4 would have to be updated.

We don't need to go into n1, n3, and n4 because nothing has changed over there.

It's the same for when a node is deleted, except that any selectors that directly applied to the node or any of its descendants must be removed.

To recalculate and update the paths of the affected nodes, we'll have to maintain weak references to those nodes along with their previously calculated paths.

(Reposted from a previous email discussion.)

comment:2 Changed on 03/02/2018 at 01:55:56 PM by mjethani

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed on 03/05/2018 at 01:31:01 PM by mjethani

  • Keywords circumvention added
  • Review URL(s) modified (diff)

comment:4 in reply to: ↑ 1 Changed on 03/05/2018 at 06:20:19 PM by mjethani

Replying to mjethani:

Actually we don't have to traverse the entire DOM every time there's a mutation just for -abp-has and -abp-contains.

I'm covering this in #6437

https://codereview.adblockplus.org/29714601/

comment:5 Changed on 03/05/2018 at 09:58:22 PM by mjethani

  • Blocked By 6446 added

comment:6 Changed on 03/07/2018 at 04:28:01 PM by mjethani

  • Blocked By 6437 added

comment:7 Changed on 03/07/2018 at 07:58:06 PM by abpbot

A commit referencing this issue has landed:
Issue 6422 - Prefer CSS selectors for -abp-has and -abp-contains

comment:8 Changed on 03/07/2018 at 07:58:26 PM by sebastian

  • Priority changed from Unknown to P3
  • Ready set

comment:9 Changed on 03/08/2018 at 06:50:15 PM by amrmak

  • Cc amrmak added

comment:10 Changed on 03/08/2018 at 10:41:42 PM by mjethani

  • Blocked By 6458 added

comment:11 Changed on 03/09/2018 at 02:51:50 AM by abpbot

A commit referencing this issue has landed:
Issue 6422 - Prefer CSS selectors for -abp-has and -abp-contains

comment:12 Changed on 03/09/2018 at 01:38:35 PM by mjethani

  • Blocked By 6459 added

comment:13 Changed on 03/09/2018 at 01:40:07 PM by mjethani

  • Blocked By 6459 removed
  • Blocking 6459 added

comment:14 Changed on 03/20/2018 at 11:11:36 AM by mjethani

  • Component changed from Core to Platform
  • Description modified (diff)
  • Review URL(s) modified (diff)

comment:15 Changed on 03/20/2018 at 11:15:19 AM by mjethani

  • Blocked By 6504 added

comment:16 Changed on 04/23/2018 at 02:37:53 PM by arthur

  • Cc arthur added

comment:17 Changed on 03/16/2019 at 03:44:48 PM by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:18 Changed on 03/16/2019 at 03:45:37 PM by mjethani

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:19 Changed on 03/16/2019 at 03:45:55 PM by mjethani

  • Resolution set to rejected
  • Status changed from reopened to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
 
Note: See TracTickets for help on using tickets.