Consider notifications without an ID to be unique
Background
In ui#562 we want to show a notification each time we encounter a problem with data storage. Furthermore, in ui#589 we want to show a notification to inform users about each major update.
Currently, the only way to avoid that such notifications are considered "shown" so that they can be shown again, is to generate a random notification ID. However, that could still result in the same ID being generated twice and it would lead to a lot of useless notification IDs to be stored in the "notificationdata" preference. If the notification ID is kept undefined, it will simply be converted into the string "undefined"
and therefore to the same problem.
Therefore we need a way to add notifications that are unaffected by the "shown" logic.
What to change
In lib/notifications.js
:
- Add a new
showNotification()
method that takes a notification object and shows the notification once immediately and unconditionally by calling all the show listeners. - In
markAsShown()
, mark the given notification as shown only if its ID matches with one of the IDs in the lists of local and remote notifications.
Integration notes
There is a new function notifications.showNotification()
that takes a notification object as usual but shows the notification immediately and unconditionally by calling each show listener in order.
There is no need to mark such notifications as shown by calling notifications.markAsShown()
from the show listener.
Changes in behavior
Only local notifications are really marked as shown now. The notifications.markAsShown()
function checks if the ID of the given notification matches any ID in the list of notifications previously added via notifications.addNotification()
and bails immediately if there is no match. If a local notification has already been removed via notifications.removeNotification()
, it will not be marked as shown. As a result, unlike in previous versions, adding the notification once again will cause it to get shown a second time. A local notification once shown should be marked as shown before it is removed, i.e. call notifications.markAsShown()
before notifications.removeNotification()
.
Hints for testers
There is no expected change in behavior in eyeo/adblockplus/adblockpluschrome> but please double-check with the dependency update issue.