Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1174 closed defect (incomplete)

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

Reported by: philll Assignee: greiner
Priority: P1 Milestone:
Module: Platform Keywords:
Cc: till, fhd, arthur, trev, saroyanm, mapx Blocked By:
Blocking: Platform: Chrome
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

Description

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.

Attachments (2)

google_ads_frame3.png (266.2 KB) - added by arthur 5 years ago.
google__frame3.png (266.2 KB) - added by arthur 5 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 5 years ago by philll

  • Cc till fhd arthur added

comment:2 Changed 5 years ago by trev

  • Cc trev added
  • Component changed from Unknown to Platform

comment:3 Changed 5 years ago by greiner

  • Owner set to greiner

comment:4 Changed 5 years ago by saroyanm

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

comment:5 Changed 5 years ago by saroyanm

  • Cc manvel@… added

comment:6 follow-up: Changed 5 years ago by 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.

comment:7 Changed 5 years ago by mapx

  • Cc saroyanm mapx added; manvel@… removed

comment:8 Changed 5 years ago by 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?

comment:9 Changed 5 years ago by greiner

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

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?

Changed 5 years ago by arthur

comment:10 in reply to: ↑ 6 Changed 5 years ago by 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?

Changed 5 years ago by arthur

comment:11 Changed 5 years ago by arthur

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

comment:12 Changed 5 years ago by 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 what I see in Chrome matches your screenshot exactly - but #google_ads_frame4.

Version 1, edited 5 years ago by trev (previous) (next) (diff)

comment:13 Changed 5 years ago by arthur

Ah alright, thanks for clarification.

comment:14 Changed 5 years ago by 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.

comment:15 Changed 5 years ago by 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.

comment:16 Changed 5 years ago by 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.

comment:17 Changed 5 years ago by 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.

comment:18 Changed 5 years ago by 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.

comment:19 Changed 5 years ago by 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.

comment:20 Changed 5 years ago by 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.

comment:21 Changed 5 years ago by arthur

  • Resolution invalid deleted
  • Status changed from closed to reopened

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?

comment:22 Changed 5 years ago by trev

  • Resolution set to invalid
  • Status changed from reopened to closed

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).

Last edited 5 years ago by trev (previous) (diff)

comment:23 Changed 5 years ago by 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.

Last edited 5 years ago by arthur (previous) (diff)

comment:24 Changed 5 years ago by 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

comment:25 Changed 5 years ago by trev

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

comment:26 Changed 5 years ago by 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.

comment:27 Changed 5 years ago by arthur

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

Note: See TracTickets for help on using tickets.