Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29836573: Issue 6809 - Implement hide-if-contains snippet (Closed)

Created:
July 23, 2018, 11:11 p.m. by Manish Jethani
Modified:
July 26, 2018, 12:18 p.m.
Reviewers:
kzar, hub
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -0 lines) Patch
M lib/content/snippets.js View 1 chunk +23 lines, -0 lines 0 comments Download

Messages

Total messages: 5
Manish Jethani
July 23, 2018, 11:11 p.m. (2018-07-23 23:11:45 UTC) #1
Manish Jethani
Patch Set 1 I'll update the Trac issue, but this is a complement to the ...
July 23, 2018, 11:15 p.m. (2018-07-23 23:15:32 UTC) #2
Manish Jethani
Since #6804 is going to be more challenging to do, we should get this snippet ...
July 26, 2018, 10:34 a.m. (2018-07-26 10:34:57 UTC) #3
hub
LGTM, but what the difference with :-abp-contains() ?
July 26, 2018, 12:05 p.m. (2018-07-26 12:05:57 UTC) #4
Manish Jethani
July 26, 2018, 12:10 p.m. (2018-07-26 12:10:51 UTC) #5
On 2018/07/26 12:05:57, hub wrote:
> LGTM, but what the difference with :-abp-contains() ?

Thanks!

:-abp-contains() runs only after the DOMContentLoaded event (see #6804), by
which time the ad has already been displayed for a second or two. When the ad is
hidden, everything shifts on the page. Overall this is a bad user experience.

Eventually we should address this in #6804 and then remove this snippet.

Powered by Google App Engine
This is Rietveld