Opened 7 months ago

Closed 7 months ago

Last modified 4 months ago

#5019 closed defect (fixed)

Anti-adblock notification not added on first-run

Reported by: greiner Assignee: jsonesen
Priority: P2 Milestone:
Module: User-Interface Keywords:
Cc: sebastian, Ross Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29390575/

Description

Environment

Windows 10
Chrome 56
Adblock Plus 1.13.1749
see https://issues.adblockplus.org/ticket/5014#comment:8

Ubuntu 16.04
Chrome 56.0.2924.87
Adblock Plus 1.13.1

How to reproduce

  1. Remove Adblock Plus if it's installed
  2. Install Adblock Plus
  3. Go to https://forums.nexusmods.com/index.php?c=411,419

Observed behaviour

Anti-adblock notification does not appear. After reloading Adblock Plus and reloading the page the notification does appear though.

Expected behaviour

Anti-adblock notification appears.

Further information

This issue is caused by the following code in antiadblockInit.js:

let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
if (subscription.lastDownload && subscription.disabled)
  addAntiAdblockNotification(subscription);

After a fresh install the anti-adblock filter list has not been downloaded yet so subscription.lastDownload is 0 whereas after reloading the extension this value is great than 0.
That code can also be found in the initial implemention of this feature.

Change History (5)

comment:1 Changed 7 months ago by sebastian

Please correct me if I'm wrong, but it seems the problem is in the code below:

  function onSubscriptionChange(subscription)
  {
    ...
    if (url in FilterStorage.knownSubscriptions && !subscription.disabled)
      addAntiAdblockNotification(subscription);
    ...
  }

  FilterNotifier.on("subscription.updated", onSubscriptionChange);
  ...

So it seems the case that the subscription hasn't been (successfully) downloaded yet is handled, but the handler seems to only add the notification if the subsciption is NOT disabled, which seems incorrect and inconsistent with the logic above.

comment:2 Changed 7 months ago by jsonesen

  • Owner set to jsonesen
  • Review URL(s) modified (diff)

comment:3 Changed 7 months ago by abpbot

comment:4 Changed 7 months ago by jsonesen

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

comment:5 Changed 4 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Now works on first run/without browser restart.

ABP 1.13.2.1785
Chrome 49 / 59 / Windows 7

Note: See TracTickets for help on using tickets.