diff --git a/js/background.js b/js/background.js index ff7761f74..b8a047b09 100644 --- a/js/background.js +++ b/js/background.js @@ -240,7 +240,7 @@ }); window.addEventListener('focus', () => Whisper.Notifications.clear()); - window.addEventListener('unload', () => Whisper.Notifications.clear()); + window.addEventListener('unload', () => Whisper.Notifications.fastClear()); Whisper.events.on('showConversation', function(conversation) { if (appView) { diff --git a/js/notifications.js b/js/notifications.js index 759af602c..b367c57b3 100644 --- a/js/notifications.js +++ b/js/notifications.js @@ -8,6 +8,7 @@ /* global Signal: false */ /* global storage: false */ /* global Whisper: false */ +/* global _: false */ // eslint-disable-next-line func-names (function() { @@ -29,12 +30,25 @@ this.on('remove', this.onRemove); this.lastNotification = null; + + // Testing indicated that trying to create/destroy notifications too quickly + // resulted in notifications that stuck around forever, requiring the user + // to manually close them. This introduces a minimum amount of time between calls, + // and batches up the quick successive update() calls we get from an incoming + // read sync, which might have a number of messages referenced inside of it. + this.fastUpdate = this.update; + this.update = _.debounce(this.update, 1000); }, onClick(conversationId) { const conversation = ConversationController.get(conversationId); this.trigger('click', conversation); }, update() { + if (this.lastNotification) { + this.lastNotification.close(); + this.lastNotification = null; + } + const { isEnabled } = this; const isAppFocused = isFocused(); const isAudioNotificationEnabled = @@ -62,7 +76,7 @@ if (status.type !== 'ok') { if (status.shouldClearNotifications) { - this.clear(); + this.reset([]); } return; @@ -148,18 +162,19 @@ }, onRemove() { console.log('Remove notification'); - if (this.length === 0) { - this.clear(); - } else { - this.update(); - } + this.update(); }, clear() { console.log('Remove all notifications'); - if (this.lastNotification) { - this.lastNotification.close(); - } this.reset([]); + this.update(); + }, + // We don't usually call this, but when the process is shutting down, we should at + // least try to remove the notification immediately instead of waiting for the + // normal debounce. + fastClear() { + this.reset([]); + this.fastUpdate(); }, enable() { const needUpdate = !this.isEnabled; diff --git a/js/read_syncs.js b/js/read_syncs.js index 5f123505a..3a248fd2e 100644 --- a/js/read_syncs.js +++ b/js/read_syncs.js @@ -24,21 +24,32 @@ message.get('source') === receipt.get('sender') ); }); - if (message) { - Whisper.Notifications.remove(message); - return message.markRead(receipt.get('read_at')).then( - function() { - this.notifyConversation(message); - this.remove(receipt); - }.bind(this) - ); - } else { - console.log( - 'No message for read sync', - receipt.get('sender'), - receipt.get('timestamp') - ); - } + const notificationForMessage = message + ? Whisper.Notifications.findWhere({ messageId: message.id }) + : null; + const removedNotification = Whisper.Notifications.remove( + notificationForMessage + ); + const receiptSender = receipt.get('sender'); + const receiptTimestamp = receipt.get('timestamp'); + const wasMessageFound = Boolean(message); + const wasNotificationFound = Boolean(notificationForMessage); + const wasNotificationRemoved = Boolean(removedNotification); + console.log('Receive read sync:', { + receiptSender, + receiptTimestamp, + wasMessageFound, + wasNotificationFound, + wasNotificationRemoved, + }); + return message + ? message.markRead(receipt.get('read_at')).then( + function() { + this.notifyConversation(message); + this.remove(receipt); + }.bind(this) + ) + : Promise.resolve(); }.bind(this) ); },