afc ads at winfuture.de aren't displayed with ABP 1.8.3.1204 while they are with ABP stable 1.8.3

[philll]

Environment

Windows 7 Enterprize
Google Chrome Version 36.0.1985.125 m
ABP 1.8.3.1204
Easylist & Easylist Germany
Adblock warning removal list
Acceptable ads allowed

How to reproduce

1. Open ​http://winfuture.de/news,82988.html

Observed behaviour

The Google ads skyscraper box right next to the main content is missing.

Expected behaviour

The Google ads skyscraper box right next to the main content should be displayed. With ABP stable 1.8.3, this works fine. Please note that the box is also missing with the same stable version but Chrome 37 beta.

[saroyanm]

Seems like the issue is appearing after current revision:
​https://hg.adblockplus.org/adblockpluschrome/rev/191f69afc9d6

[greiner]

There's the filter ###google_ads_frame4 in EasyList Germany but no filter winfuture.de#@##google_ads_frame4 in the Acceptable Ads filterlist that would disable it so the expected outcome is that the ad is hidden.

The commit @manvel is refering to was about fixing element hiding in anonymous frames which is why my current assumption is that the current behavior is the intended one and that the previous behavior was a bug (i.e. the ad was shown because element hiding wasn't working).

However, the ad is also shown in Firefox because the hiding filter is not applied. If you disable any hiding filter using the blockable items list, the filter is applied and the ad gets hidden. Therefore it appears that this could be a separate issue in Firefox which I'm going to investigate next.

[trev]

Please note that element hiding currently works differently in Firefox if multiple rules apply to the same element. There, one of the rules "wins" and it is sufficient to have an exception for this one. In Chrome however we require exceptions for all matching rules. #521 should fix this implementation detail, Firefox will behave the same as Chrome then.

This is most likely why you see different behavior in Firefox. From the sound of it, this issue should be closed as invalid and resolved in the Acceptable Ads list?

[greiner]

Thanks for the background info on that.

@Arthur Could you please check the whitelist filters for this ad again in both Firefox and Chrome and add the ones that are missing?

[arthur]

Replying to greiner:

There's the filter ###google_ads_frame4 in EasyList Germany but no filter winfuture.de#@##google_ads_frame4 in the Acceptable Ads filterlist that would disable it so the expected outcome is that the ad is hidden.

If I try it with Chrome 36 and ABP 1.8.3, I'm getting a google_ads_frame3 ID. In EasyList there is ###google_ads_frame3 but there is no exception rule like winfuture.de#@##google_ads_frame3 in the whitelist, still the ad shows with the release build of ABP but not in the dev build with Chrome 36.

I know it works when I add such a whitelist in the dev build but why do we suddenly need it or better: why doesn't the release build hide the ad like the dev build?

[arthur]

Whoops, we're blocking the first attachment. :D Please take a look on google__frame3.png instead.

[trev]

As greiner said above, the devbuild fixes #581 - before this fix no element hiding rules were applied in the frame in question (acceptable ads or not), that was a bug.

Oh, and the frame ID seems to depend on the page. On main page I see #google_ads_frame3, on article pages I see #google_ads_frame4.

[arthur]

Ah alright, thanks for clarification.

[philll]

As discussed in person: We will have a look at the script injecting all the iframes and contents to see what kind of IDs are possible. Depending on that outcome, we might simply create one partially wildcarded filter for both, hiding and whitelisting, that matches something like google_ads_frame* In terms of performance, that might even be better than having n filters matching only one element in the end.

[greiner]

The number at the end of #google_ads_frame is taken from the variable google_unique_id which is created by the AFC script and is used to identify the ad unit. It appears to be an incrementing counter so it's 1 for the first ad, 2 for the second ad and so on. Therefore there doesn't appear to be an upper limit.

[trev]

@philll: With CSS, rules based on anything other than literal ID/class names always have bad performance. So if we can avoid wildcards we definitely should. In the end, what's important are the rules in EasyList that we create exceptions for. These go from #google_ads_frame1 to #google_ads_frame7 meaning that seven exception rules will be necessary here. These can be reused for other domains as well.

[philll]

@trev: as greiner just noted, there is no upper limit of the index number of those iframe IDs, meaning that in the case of eg gutefrage.net, where one Google ad follows each user comment, 7 will be exceeded quite easily. That of course is also true for the entries in Easylist.

[arthur]

@trev: This sounds like adding global exception rules to the whitelist. I'd rather avoid that and add specific ones for all sites using AFC ads instead. Yes, this will be quite a lot but with global ones any site could take advantage of it.

[trev]

@arthur: What I meant are rules like winfuture.de,foo.example.com,bar.example.com#@##google_ads_frame4 rather than three individual rules - one for each site.

[arthur]

Ah right. I wasn't thinking of combining all domains since we put all necessary whitelists for a page into one section so all its rules can easily be removed at once if necessary. Your suggestion seems to be the best currently.

[arthur]

Looks like this is happening again in the current versions of Chrome and ABP.

Chrome 37.0.2062.102 m on Windows 8.1 Pro and Adblock Plus 1.8.3.

Works fine with the dev build.

Could someone please look into it?

[trev]

From all I can tell, winfuture.de simply stopped serving the ads we whitelist - that skyscraper ad isn't visible in Firefox either, and when I disable Adblock Plus I see an image banner that clearly isn't acceptable. I can only see a 660x75 AdSense block whitelisted on both main page and article pages, on the article pages it is being hidden by the ##.adsbygoogle filter however (not sure why it doesn't appear in the blockable items list, created #1289 on it).

[arthur]

I do see the skyscraper ad in FF and Chrome 37 with the dev build. Note that it is triggered by a detection filter in the AA whitelist.

Thomas just looked into it and will post his findings.

[greiner]

The issue here was caused by element collapsing which is hiding the ad by setting display: none !important on one of the ad's iFrames. Usually, only frames that have failed to load are being collapsed but in this case it was also applied to a visible ad.

That problem appears to be fixed with ​https://hg.adblockplus.org/adblockpluschrome/rev/191f69afc9d6

[trev]

As noted in #1289, the 660x75 AdSense block is being hidden by the filter winfuture.de###news_content > .o_hidden > .singleblock_news_top.

[greiner]

We might be talking about two different issues so just to clarify: The issue I have been looking into was that in the dev build the skyscraper ad on the right was shown, in the stable build it was hidden.

[arthur]

Are we going to have a bugfix release next week with the fix Thomas mentioned?