Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#6618 closed defect (rejected)

Element hiding emulation uses inline styles for plain selectors

Reported by: mjethani Assignee:
Priority: Unknown Milestone:
Module: Core Keywords: circumvention
Cc: kzar, hfiguiere Blocked By:
Blocking: #6437 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29760631/

Description (last modified by mjethani)

Environment

ABP 3.0.3 on Chrome 66

How to reproduce

  1. Add the filter example.com#?#p
  2. Visit https://example.com/
  3. Open the DevTools inspector to examine the DOM

Observed behaviour

The <p> elements are hidden by setting the style="..." attribute

Expected behaviour

The <p> elements should be hidden using a CSS selector

Notes

The reason this matters is that plain selectors are not dependent on DOM mutations, so we don't process these (#6437) when there's a mutation. If we use CSS selectors in the first place there's no need to process these selectors each time there's a mutation.

Change History (8)

comment:1 Changed 2 years ago by mjethani

  • Blocking 6437 added
  • Cc kzar hfiguiere added
  • Description modified (diff)

comment:2 Changed 2 years ago by hfiguiere

No. That's the correct behaviour for a #?# filter. Simple element hiding with style sheets should be a filter of type ##.

comment:3 Changed 2 years ago by mjethani

What is the rationale for this? It seems like a no-brainer to use a CSS selector in this case instead of calling querySelectorAll each time there's a mutation.

One more thing to note is that a CSS selector would also apply to shadow DOMs whereas querySelectorAll only works on the light DOM.

comment:4 Changed 2 years ago by mjethani

Also by the way since we have switched to user style sheets this is kind of moot, we are already using CSS selectors here if tabs.removeCSS is available.

comment:5 Changed 2 years ago by hfiguiere

This was issue #5314

This was before user stylesheets in Chrome.

comment:6 Changed 2 years ago by mjethani

OK, this makes sense. Anyway if we make the change for #6610 then it's going to go the other way. I'll close this.

comment:7 Changed 2 years ago by mjethani

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

comment:8 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.