Opened 12 days ago

Last modified 6 days ago

#7337 reviewing change

Create :-abp-contains-visible() advanced selector

Reported by: hfiguiere Assignee: hfiguiere
Priority: Unknown Milestone:
Module: Unknown Keywords: circumvention
Cc: arthur, mapx, amr Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/30024560

Description (last modified by hfiguiere)

Background

We need to check for the visibility of text content as some website have hidden text, that is still present in Node.textContent

What to change

  • in lib/content/elemHideEmulation.js add an implementation of ContainsSelector that will check the text visibility.
  • this will use the :-abp-contains-visible() pseudo selector syntax.

Text visibility is done with CSS. This include 0 font-size, transparent (opacity = 0), same foreground and background colour, visibility and display. We should review what are the other possibilities.

Notes

I don't think this needs to be a snippet.

Hint for testers

TBD

Change History (10)

comment:1 Changed 12 days ago by hfiguiere

  • Owner set to hfiguiere

comment:2 Changed 12 days ago by hfiguiere

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

comment:3 Changed 12 days ago by hfiguiere

  • Description modified (diff)

comment:4 follow-up: Changed 12 days ago by agiammarchi

Just thinking out loudly, negative z-index, absolute/fixed position, transform3d, are also a way to make some text "disappear". Since they play with classes too, a font family that contains only zero spaced chars could work too.

visibility is, on the other hand, the only one that keeps the rendered size so it might be worth checking that, but it's not more interesting than others techniques.

comment:5 in reply to: ↑ 4 Changed 12 days ago by hfiguiere

Replying to agiammarchi:

Just thinking out loudly, negative z-index,

Negative z-index doesn't necessarily hide, so the heuristics are a bit more complex.

absolute/fixed position, transform3d, are also a way to make some text "disappear". Since they play with classes too, a font family that contains only zero spaced chars could work too.

Maybe we can consider that if one of the element dimension is 0 it is not visible. Or a negative position.

visibility is, on the other hand, the only one that keeps the rendered size so it might be worth checking that, but it's not more interesting than others techniques.

visibility is already handled. Note that innerText vs textContent already handle that difference (albeit we use the later in :-abp-contains)

comment:6 Changed 12 days ago by arthur

  • Cc arthur added

comment:7 Changed 10 days ago by mapx

  • Cc mapx added

comment:8 Changed 7 days ago by amrmak

  • Cc amr added

comment:9 Changed 7 days ago by mjethani

While I like the idea behind this, I am not too sure of the syntax. Maybe we don't need a new pseudoclass but can handle it with the same pseudoclass but with some enhancements to the syntax within the parentheses.

e.g.

div:-abp-contains(='Hello world')

Searches for visible text "Hello world".

div:-abp-contains(!'Hello world')

Searches for hidden text "Hello world".

div:-abp-contains('Hello world')

Searches for text "Hello world" regardless of visibility.

I'm just thinking out loud.

comment:10 Changed 6 days ago by hfiguiere

I don't think I agree with that syntax. It is less clear for the filter author, it is more complicated for us to implement. What's the problem of having a new pseudo class?

Note: See TracTickets for help on using tickets.