Opened 3 years ago

Closed 3 years ago

#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
http://codereview.adblockplus.org/6202090942627840

Description

Environment

Firefox 38.0
Adblock Plus 2.6.9.3939

How to reproduce

  1. Go to about:config
  2. Reset the preference extensions.adblockplus.notificationdata
  3. 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

Change History (7)

comment:1 Changed 3 years ago by sebastian

  • Cc sebastian added
  • Priority changed from Unknown to P1

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.

comment:2 Changed 3 years ago 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 3 years ago 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 3 years ago by sebastian

  • Owner set to sebastian

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

Yup, just waiting for Wladimir's approval - both are core changes.

comment:7 Changed 3 years ago by fhd

  • Milestone set to Adblock-Plus-for-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.