Opened on 12/25/2014 at 08:17:03 PM
Last modified on 10/19/2016 at 05:32:07 PM
#1737 reviewing defect
ABP 1.3 for IE sometimes blocks navigation to the ad from Google search results
| Reported by: | mapx | Assignee: | Mailkov |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | Adblock-Plus-for-Internet-Explorer | Keywords: | |
| Cc: | oleksandr, sergz, erikcramer | Blocked By: | |
| Blocking: | Platform: | Internet Explorer | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by sergz)
Environment
win 7, IE 10, ABP 1.3
win 8.1, IE 11, ABP 1.4
How to reproduce
- Make sure Ad Block 1.3 is Enabled on http://www.google.com and Acceptable Ads are enabled.
- Enter search term that leads to ads.
- Right click on the displayed ad and select Open in New Tab
- Observe the site advertised opening fine
- Disable Ad Block 1.3 on http://www.google.com.
- Enter search term that leads to ads.
- Right click on ads and select Open in New Tab.
- Observe that sometimes links from ads are only opening blank pages.
Looks like sometimes the google ad leads to googleadservices.com, which then redirects to the actual website advertised. Other times it leads to just an empty page with the URL google.com.
from
https://adblockplus.org/forum/viewtopic.php?f=16&t=26978
Observed
The newly opened tab is a blank page.
Expected
The correctly filled page as we don't block root page.
The record from log
Blocked SUBDOCUMENT - http://www.google.de/aclk?sa=l&ai=Cfvc2X_aHVYTTDMbi7gbksZCoCLz9uLQH...%26lpid%3D%26aceid%3D%26hkz%3D
Attachments (0)
Change History (18)
comment:2 Changed on 12/26/2014 at 12:06:42 AM by mapx
- Summary changed from ABP 1.3 for IE - strange behaviour when "disable on..." to ABP 1.3 for IE - wrong behaviour when "disable on..."
comment:3 Changed on 01/02/2015 at 03:57:28 PM by mapx
- Cc sergz added
comment:4 Changed on 01/21/2015 at 12:17:37 PM by oleksandr
- Priority changed from Unknown to P3
- Ready set
comment:5 Changed on 03/18/2015 at 09:43:58 AM by Mailkov
I would like to work on this defect.
comment:6 Changed on 03/18/2015 at 02:12:18 PM by oleksandr
- Owner set to Mailkov
Sounds great, thanks! Can you reproduce the bug for all links or only for ad links? I can reproduce it with v 1.4, but not with the current tip.
comment:7 Changed on 05/04/2015 at 10:17:34 AM by Mailkov
Hi, I'm back !!!
I tested it both v.1.4 and current tip ... The disable function seems to not work .
I'm investigating on it.
comment:8 Changed on 05/13/2015 at 09:34:53 AM by Mailkov
comment:9 Changed on 05/13/2015 at 12:08:53 PM by philll
- Ready unset
- Verified working unset
Could someone please clean up this issue with a proper observed and expected behaviour section? Unset ready until this is done and it's fully reproducible.
comment:10 Changed on 06/16/2015 at 01:07:38 AM by oleksandr
- Description modified (diff)
- Ready set
comment:11 Changed on 06/16/2015 at 01:34:03 AM by oleksandr
- Summary changed from ABP 1.3 for IE - wrong behaviour when "disable on..." to ABP 1.3 for IE sometime blocks navigation to the ad from Google search results
comment:12 Changed on 06/16/2015 at 01:34:22 AM by oleksandr
- Summary changed from ABP 1.3 for IE sometime blocks navigation to the ad from Google search results to ABP 1.3 for IE sometimes blocks navigation to the ad from Google search results
comment:13 Changed on 06/22/2015 at 11:55:47 AM by sergz
- Description modified (diff)
comment:14 follow-up: ↓ 16 Changed on 06/22/2015 at 02:59:24 PM by sergz
The request of the newly opened tab is being executed in a new process and in WBPassthruSink::BeginningTransaction tab is nullptr, m_boundDomain is empty and accept header is text/html, application/xhtml+xml, */*, so the "content type" is subdocument. (We have such configurations for valid to block page parts.) But the exception rule is @@||www.google.de^$document, as the result it's not whitelisted. In general the trouble is that we cannot say that it's "document" instead of "subdocument". The solution is to check the current number of tabs, if it's zero then assume that it's a document.
comment:15 Changed on 06/22/2015 at 05:36:17 PM by sergz
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:16 in reply to: ↑ 14 ; follow-up: ↓ 17 Changed on 06/29/2015 at 06:20:25 PM by Mailkov
Replying to sergz:
The solution is to check the current number of tabs, if it's zero then assume that it's a document.
Can you explain better?
Why when is number of tabs zero we can assume that it's a 'document'?
Tabs in this situation is never zero in my opinion ...
comment:17 in reply to: ↑ 16 Changed on 06/30/2015 at 03:41:01 PM by sergz
- Tester set to Unknown
Replying to Mailkov:
Replying to sergz:
The solution is to check the current number of tabs, if it's zero then assume that it's a document.
Can you explain better?
Why when is number of tabs zero we can assume that it's a 'document'?
Tabs in this situation is never zero in my opinion ...
According to blogs.msdn.com/b/askie/archive/2009/03/09/opening-a-new-tab-may-launch-a-new-process-with-internet-explorer-8-0.aspx
In general, new processes are created until the TabProcGrowth number is met, and then tabs are load balanced across the tab processes.
So, the idea is that if there is no referrer and there is only one tab then we can assume that it's a first request of the page, thus document.
We are currently adding the current instance into CPluginClass::s_threadinstances when the tab state is changed which on practice happens after the downloading of a new tab page is initiated. Beside apparent race condition there is a number of tabs starting from which it does not work.
See also comments in review https://codereview.adblockplus.org/29319002/.
comment:18 Changed on 10/19/2016 at 05:32:07 PM by fred
- Cc erikcramer added
I tested the issue (reported in the forum) on IE 11, and I can reproduce it partially: the links from the normal search can be opened in a new tab thru right click.