Opened 20 months ago

Closed 7 months ago

#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()).

Change History (19)

comment:1 follow-up: Changed 20 months ago 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 20 months ago by mjethani

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

comment:3 Changed 20 months ago by mjethani

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

comment:4 in reply to: ↑ 1 Changed 20 months ago 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 20 months ago by mjethani

  • Blocked By 6446 added

comment:6 Changed 20 months ago by mjethani

  • Blocked By 6437 added

comment:7 Changed 20 months ago by abpbot

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

comment:8 Changed 20 months ago by sebastian

  • Priority changed from Unknown to P3
  • Ready set

comment:9 Changed 20 months ago by amrmak

  • Cc amrmak added

comment:10 Changed 20 months ago by mjethani

  • Blocked By 6458 added

comment:11 Changed 20 months ago by abpbot

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

comment:12 Changed 20 months ago by mjethani

  • Blocked By 6459 added

comment:13 Changed 20 months ago by mjethani

  • Blocked By 6459 removed
  • Blocking 6459 added

comment:14 Changed 19 months ago by mjethani

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

comment:15 Changed 19 months ago by mjethani

  • Blocked By 6504 added

comment:16 Changed 18 months ago by arthur

  • Cc arthur added

comment:17 Changed 7 months ago by mjethani

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

comment:18 Changed 7 months ago by mjethani

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:19 Changed 7 months ago by mjethani

  • Resolution set to rejected
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.