Opened on 05/27/2015 at 01:01:30 PM
Closed on 06/08/2015 at 10:43:30 AM
#2592 closed defect (fixed)
Regression: URL-specific notifications are broken
| Reported by: | greiner | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P1 | Milestone: | Adblock-Plus-2.6.10-for-Firefox |
| Module: | Core | Keywords: | |
| Cc: | trev, sebastian | Blocked By: | |
| Blocking: | Platform: | Firefox | |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): |
http://codereview.adblockplus.org/5230613363687424 |
||
Description
Environment
Firefox 38.0
Adblock Plus 2.6.9.3939
How to reproduce
- Go to about:config
- Reset the preference extensions.adblockplus.notificationdata
- Go to wwwok.at
Observed behavior
No notification is shown
Expected behavior
Notification is shown that asks whether anti-adblock messages should be hidden
Background
This is a regression that was introduced in #2503 and is affecting this specific notification because currently, we're constructing the filters that are used to trigger the anti-adblock notification as follows: "||" + domain + "^"
What to change
Append $document to each of the filters that are dynamically constructed in lib/antiadblockInit.js~addAntiAdblockNotification
Attachments (0)
Change History (7)
comment:1 Changed on 06/04/2015 at 09:43:36 PM by sebastian
- Cc sebastian added
- Priority changed from Unknown to P1
comment:2 Changed on 06/05/2015 at 10:42:09 AM by fhd
- Review URL(s) modified (diff)
#2503 also broke the tests for URL-specific notifications. Fixed those, since I need the notification tests to pass for what I'm working on.
comment:3 Changed on 06/05/2015 at 11:32:08 AM by sebastian
- Ready set
- Review URL(s) modified (diff)
- Status changed from new to reviewing
And here comes the patch fixing the notification.
comment:4 Changed on 06/05/2015 at 11:32:36 AM by sebastian
- Owner set to sebastian
comment:5 Changed on 06/05/2015 at 11:45:16 AM by sebastian
Landed fix for the notification:
https://hg.adblockplus.org/adblockplus/rev/0f8feec93b61
@fhd: Please close the issue when you have merged the test changes.
comment:6 Changed on 06/08/2015 at 09:20:10 AM by fhd
Yup, just waiting for Wladimir's approval - both are core changes.
comment:7 Changed on 06/08/2015 at 10:43:30 AM by fhd
- Milestone set to Adblock-Plus-for-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
Test fix landed as well: https://hg.adblockplus.org/adblockplustests/rev/e7fd9016aab1
For reference, since the dependency update (#2639) this issue is visible on Chrome/Opera/Safari as well now. I suppose nobody objects if I set this issue to P1, as it pretty much blocks us from having a release on 4 platforms now.