Opened 6 months ago

Closed 3 months ago

Last modified 2 months ago

#5864 closed change (fixed)

Maintain at most one style sheet for element hiding emulation

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone: Adblock-Plus-3.0.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, hfiguiere, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29575739/
https://codereview.adblockplus.org/29685561/

Description (last modified by sebastian)

Background

Currently -abp-properties causes multiple style sheets, or multiple sets of selectors (in the case of styles injected into the DOM), to be added to the document as the DOM is updated and new elements are affected.

This HTML, for example:

<style>
  .greeting1 { color: blue }
</style>
<div>
  <h1 class="greeting1">Hello</h1>
  <h1 class="greeting2">Hi</h1>
</div>
<script>
  setTimeout(() =>
  {
    let style = document.createElement("style");
    style.innerHTML = ".greeting2 { color: blue }";
    document.head.appendChild(style);
  },
  1000);
</script>

With the filter localhost#?#h1:-abp-properties(color: blue), the first H1 with the greeting1 class ends up with two sets of selectors applying to it:

h1.greeting1 { display: none !important }
h1.greeting1, h1.greeting2 { display: none !important }

The first set is redundant here.

If style sheets are removed from the document, the elements affected by the change still remain hidden, thus potentially breaking the page's functionality.

What to change

Remove previous stylesheet before adding a new one.

Change History (13)

comment:1 Changed 6 months ago by mjethani

  • Cc sebastian dave added

comment:2 Changed 6 months ago by mjethani

  • Review URL(s) modified (diff)

comment:3 Changed 6 months ago by hfiguiere

  • Cc hfiguiere added

comment:4 Changed 6 months ago by mapx

  • Cc kzar added; dave removed

comment:5 Changed 6 months ago by sebastian

  • Priority changed from Unknown to P2
  • Ready set

comment:6 Changed 6 months ago by sebastian

  • Description modified (diff)

comment:7 Changed 5 months ago by mjethani

  • Owner set to mjethani

comment:8 Changed 3 months ago by abpbot

A commit referencing this issue has landed:
Issue 5864 - Remove previous style sheet before adding one

comment:9 Changed 3 months ago by mjethani

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from new to closed

comment:10 Changed 3 months ago by abpbot

A commit referencing this issue has landed:
Issue 5864 - Remove previous style sheet before adding one

comment:11 Changed 3 months ago by abpbot

A commit referencing this issue has landed:
Issue 5864 - Specify CSS group name for standard filters

comment:12 Changed 3 months ago by mjethani

  • Review URL(s) modified (diff)
  • Summary changed from Maintain one at most one style sheet for element hiding emulation to Maintain at most one style sheet for element hiding emulation

comment:13 Changed 2 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Looks to work as expected (one set of selectors applied to the page/one stylesheet). Thank you for the example HTML/script.

ABP 3.0.2.1968
Chrome 64 / 49 / Canary (66) / Windows 7
Opera 49 / 36 / Beta (52) / Windows 7
Firefox 58 / 51 / Beta (59) / Windows 7
Firefox Mobile 57 / Android 7.0.1

Note: See TracTickets for help on using tickets.