Opened on 08/30/2016 at 09:08:49 PM
Closed on 09/05/2016 at 05:11:31 PM
Last modified on 10/25/2016 at 02:55:24 AM
#4386 closed defect (fixed)
Document domain determined incorrectly after a redirect
| Reported by: | mapx | Assignee: | trev |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-1.12.4-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | trev, kzar, sebastian, greiner | Blocked By: | |
| Blocking: | Platform: | Chrome | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | yes |
| Review URL(s): |
https://codereview.adblockplus.org/29350357/ |
||
Description (last modified by trev)
Environment
windows 10
chrome 53
ABP 1.12.2
How to reproduce
- whitelist theatlantic.com
- go to http://theatln.tc/2bPtk9V
Observed behaviour
- the link above will redirect to http://www.theatlantic.com/business/archive/2016/08/californias-smart-new-retirement-plan-and-the-industry-that-opposes-it/498038/?utm_source=atlfb
- the page is not whitelisted
Expected behaviour
- the page should be whitelisted, the ads visible, should be exactly as in ABP for firefox
If I remove theatlantic.com from whitelisted domains and add theatln.tc
the landing page: http://www.theatlantic.com/business/archive/2016/08/californias-smart-new-retirement-plan-and-the-industry-that-opposes-it/498038/?utm_source=atlfb
...will be whitelisted !
on forum: https://adblockplus.org/forum/viewtopic.php?f=10&t=48270
Background
chrome.webNavigation.onBeforeNavigate isn't being triggered after the redirect, it's merely being triggered for the original URL. It seems that we are using the wrong callback here - we shouldn't get the original URL in the first place because there is no document associated with it. Also, I suspect that onBeforeNavigate doesn't actually mean that the previous document is unloaded or that the navigation will definitely occur.
What to change
The chrome.webNavigation.onCommitted callback seems more appropriate here, as this one will only fire when the current document is replaced - which is what we need.
Attachments (0)
Change History (8)
comment:1 Changed on 08/31/2016 at 02:24:14 PM by trev
- Priority changed from Unknown to P1
- Ready set
- Summary changed from different behaviour in ABP firefox / chrome regarding whitelisting domains to Document domain determined incorrectly after a redirect
comment:2 Changed on 08/31/2016 at 02:27:20 PM by trev
For reference, I reproduced this with the latest repository state on Chrome 52 and Mac OS X. I cannot really imagine this not being a regression however, probably caused by some internal Chrome change.
comment:3 Changed on 08/31/2016 at 02:42:11 PM by trev
- Description modified (diff)
- Owner set to trev
comment:5 Changed on 09/05/2016 at 05:10:46 PM by abpbot
A commit referencing this issue has landed:
Issue 4386 - Fixed determining document domain, particularly after being redirected
comment:6 Changed on 09/05/2016 at 05:11:31 PM by trev
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from new to closed
comment:7 Changed on 10/22/2016 at 02:40:16 PM by sebastian
- Priority changed from P1 to P2
comment:8 Changed on 10/25/2016 at 02:55:24 AM by rraceanu
- Verified working set
Issue no longer occurs, domains are whitelisted after redirect, verified on ABP version 1.12.2.1670, Chrome 41,45,51,53,54.
I can confirm this, seems to be a timing issue. The requests are being attributed to the wrong document URL, also visible in our Developer Tools panel (document domain is being shown as theatln.tc) - but only if I open Developer Tools when the page is loading already, having it open from the start makes things work correctly.