Skip to content

Improve element hiding empty check to avoid potentially hiding visible content #319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

dharb
Copy link
Contributor

@dharb dharb commented Mar 14, 2023

Asana link: https://app.asana.com/0/1203557590179903/1204090946699947/f
Corresponding privacy test page update: duckduckgo/privacy-test-pages#130

While we try to be precise in the selector rules we use for element hiding, we've run into a few cases now where we accidentally hid visible content that matched an ad selector. This PR adds several guardrails to prevent hiding visible content even when elements match common ad selectors.

Specifically, this PR:

  • avoids hiding elements that contain various media tags such as <embed>, <object>, <audio>, and <map>
  • avoids hiding elements that contain form tags such as <form>, <input>, <textarea>, <select>, <option>, and <button>
  • avoids hiding elements that contain real images - we previously weren't counting images as visible content since ad containers frequently include tracking pixels and small ad logos (like adchoices). Now we the size of contained images and don't hide when they're > 20px width or height.

@dharb dharb force-pushed the dharb/better-element-hiding-heuristics branch from 616c25e to 3d308fd Compare March 14, 2023 23:10
Copy link
Contributor

@jonathanKingston jonathanKingston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the selectors configurable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants