Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#2192 closed change (fixed)

Add notification opt-out mechanism

Reported by: greiner Assignee: greiner
Priority: P2 Milestone: Adblock-Plus-2.6.10-for-Firefox
Module: Core Keywords:
Cc: trev Blocked By:
Blocking: #2191, #2193, #2195, #2237 Platform: Unknown
Ready: yes Confidential: no
Tester: Ross Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5270660880269312/

Description (last modified by trev)

Background

We want to provide a way to opt-out of notifications.

What to change

  • Add a preference "notifications_showui" as a boolean value with a default value of false.
  • Add a preference "notifications_ignoredcategories" as an array containing the categories of notifications that the user opted-out of.
  • Expose a function in lib/Notification.js for toggling (i.e. adding/removing) categories in the list.
  • Set Prefs.notifications_showui to true when a category is added to the list.
  • Ignore all notifications in Notification.getNextToShow() (except those of type "critical") if Prefs.notifications_ignoredcategories contains "*". Additionally, ignore a notification if its category is listed in Prefs.notifications_ignoredcategories.

Change History (13)

comment:1 Changed 5 years ago by greiner

  • Blocked By 2195 added

comment:2 Changed 5 years ago by greiner

  • Blocking 2193 added

comment:3 Changed 5 years ago by greiner

  • Blocked By 2195 removed
  • Blocking 2195 added

comment:4 Changed 4 years ago by greiner

  • Description modified (diff)
  • Verified working unset

comment:5 Changed 4 years ago by greiner

  • Owner set to greiner

comment:6 Changed 4 years ago by greiner

  • Blocking 2237 added

comment:7 follow-up: Changed 4 years ago by trev

  • Cc trev added

I don't understand the purpose of the notifications_showcheckbox preference. According to #2193 it is meant to toggle the notifications UI under Options. Shouldn't that UI generally be visible whenever notifications_ignorecategories is non-empty?

comment:8 in reply to: ↑ 7 Changed 4 years ago by greiner

Replying to trev:

I don't understand the purpose of the notifications_showcheckbox preference. According to #2193 it is meant to toggle the notifications UI under Options. Shouldn't that UI generally be visible whenever notifications_ignorecategories is non-empty?

Not necessarily because that would mean that the option will disappear as soon as you click on it to opt-in again with no way to revert your action (except waiting for the next notification to show up).

Besides that, the assumption is that a user who found the menu item expects it to be there again in the future, no matter how often he/she used it or not. If that's not the case I'd consider it to be a bad user experience. We can, of course, involve Sven here, if you want.

comment:9 Changed 4 years ago by trev

  • Description modified (diff)
  • Ready set

Fair enough. I changed the preference names, and renamed the catch-all category into "*".

comment:10 Changed 4 years ago by greiner

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:11 Changed 4 years ago by greiner

  • Milestone set to Adblock-Plus-for-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:12 Changed 4 years ago by Ross

  • Tester set to Ross

I've been testing this (+ related tickets) and it appears to be working correctly as described in Chrome and Opera so far. Will be checking Safari and Firefox today.

comment:13 Changed 4 years ago by Ross

Add a preference "notifications_showui" as a boolean value with a default value of false.

Done.

Add a preference "notifications_ignoredcategories" as an array containing the categories of notifications that the user opted-out of.

Done.

Expose a function in lib/Notification.js for toggling (i.e. adding/removing) categories in the list.

Done

Set Prefs.notifications_showui to true when a category is added to the list.

Done / Working.

Ignore all notifications in Notification.getNextToShow() (except those of type "critical") if Prefs.notifications_ignoredcategories contains "*".

Done / Working.

Additionally, ignore a notification if its category is listed in Prefs.notifications_ignoredcategories.

I'm not sure how this is supposed to work as if you add a category = foo to a notification it is not included in the response to /notification.json. Preperation for a category key later?

ABP 2.6.9.3966 / Firefox 39.0 / Windows 7 x64

Note: See TracTickets for help on using tickets.