Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#4486 closed defect (fixed)

Broken detection whether a document (root frame) is whitelisted or not

Reported by: oleksandr Assignee: sergz
Priority: P1 Milestone: Adblock-Plus-for-Internet-Explorer-1.6
Module: Adblock-Plus-for-Internet-Explorer Keywords:
Cc: oleksandr, sergz, philll Blocked By:
Blocking: Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Rraceanu Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29355411/

Description (last modified by sergz)

Environment

ABP for IE, master and issue-119-inject-css bookmarks.

How to reproduce

  1. Go to http://google.com
  2. Search for ad heavy term like 'flower shop' or 'buy iphone'
  3. Observe no Acceptable Ads
  4. Check Acceptable Ads switch in settings to make sure they are enabled
  5. Disable ABP on google.com and refresh the page to make sure the ad is actually served.

Expected behaviour

Acceptable Ads should be shown based on a setting on the settings page.

Additional info

It affects only certain filters, see comment #6.

Change History (9)

comment:1 Changed 3 years ago by oleksandr

  • Description modified (diff)

comment:2 Changed 3 years ago by sergz

  • Owner set to sergz

comment:3 Changed 3 years ago by sergz

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

Short fix for the present.

comment:5 Changed 3 years ago by oleksandr

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:6 Changed 3 years ago by sergz

  • Cc philll added
  • Description modified (diff)
  • Summary changed from Acceptable Ads switch is broken on google.com with CSS injection enabled to Broken detection whether a document (root frame) is whitelisted or not

The issue is that we check whether the document is whitelisted (as well as element hiding) a wrong way.
For example, there is a filter @@||www.google.de^$elemhide,~third-party (on my machine, google redirects to google.de, but it does not matter) and in the engine we call Main.cpp::GetWhitelistingFilter("https://www.google.de/#q=iphone", empty vector, CONTENT_TYPE_ELEMHIDE) which calls GetWhitelistingFilter(urlArg, "", type) and it returns an empty string. However, it should use URL as document URL instead of an empty string, thus call GetWhitelistingFilter(urlArg, urlArg, type).

It has been introduced here.

It does not affect for instance filters like @@||example.org^$elemhide which are created when a web site is manually whitelisted.

comment:7 Changed 3 years ago by oleksandr

  • Milestone set to Adblock-Plus-for-Internet-Explorer-Next

comment:8 Changed 3 years ago by rraceanu

  • Tester changed from Unknown to Rraceanu

comment:9 Changed 3 years ago by rraceanu

  • Verified working set

Change successfully implemented, AA ads are shown on whitelisted websites, verified on IE 10 Windows 8, IE 11 Windows 10, ABP version 1.5.856

Note: See TracTickets for help on using tickets.