From 93696729c6b51e1c86c95ad39ceac8ac3cde8a05 Mon Sep 17 00:00:00 2001 From: Seth Nickell Date: Wed, 15 May 2019 18:27:37 -1000 Subject: [PATCH] notifications: Disable default permission pop up. Prior to this commit, we'd put up the green "Enable desktop notifications" bar on page load AND the first time a desktop notification worthy message was received, it would attempt to notify, automatically triggering a browser permission popup (the same one as clicking the green bar results in). Now, desktop notifications are not attempted at all until the green bar is clicked. Additionally Firefox and Webkit browser-specific checks are made more uniform and done at the same point. Tested written by YashRE42. Fixes #11504. --- frontend_tests/node_tests/notifications.js | 12 +++++++---- static/js/notifications.js | 25 +++++++--------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/frontend_tests/node_tests/notifications.js b/frontend_tests/node_tests/notifications.js index 1a8f9af965..d50e8fc8cc 100644 --- a/frontend_tests/node_tests/notifications.js +++ b/frontend_tests/node_tests/notifications.js @@ -12,6 +12,10 @@ set_global('page_params', { is_admin: false, realm_users: [], }); +const _navigator = { + userAgent: 'Mozilla/5.0 AppleWebKit/537.36 Chrome/64.0.3282.167 Safari/537.36', +}; +set_global('navigator', _navigator); zrequire('muting'); zrequire('stream_data'); @@ -208,7 +212,7 @@ run_test('basic_notifications', () => { }; // Send notification. - notifications.process_notification({message: message_1, webkit_notify: true}); + notifications.process_notification({message: message_1, desktop_notify: true}); n = notifications.get_notifications(); assert.equal('Jesse Pinkman to general > whatever' in n, true); assert.equal(Object.keys(n).length, 1); @@ -223,7 +227,7 @@ run_test('basic_notifications', () => { // Send notification. message_1.id = 1001; - notifications.process_notification({message: message_1, webkit_notify: true}); + notifications.process_notification({message: message_1, desktop_notify: true}); n = notifications.get_notifications(); assert.equal('Jesse Pinkman to general > whatever' in n, true); assert.equal(Object.keys(n).length, 1); @@ -231,14 +235,14 @@ run_test('basic_notifications', () => { // Process same message again. Notification count shouldn't increase. message_1.id = 1002; - notifications.process_notification({message: message_1, webkit_notify: true}); + notifications.process_notification({message: message_1, desktop_notify: true}); n = notifications.get_notifications(); assert.equal('Jesse Pinkman to general > whatever' in n, true); assert.equal(Object.keys(n).length, 1); assert.equal(last_shown_message_id, message_1.id); // Send another message. Notification count should increase. - notifications.process_notification({message: message_2, webkit_notify: true}); + notifications.process_notification({message: message_2, desktop_notify: true}); n = notifications.get_notifications(); assert.equal('Gus Fring to general > lunch' in n, true); assert.equal('Jesse Pinkman to general > whatever' in n, true); diff --git a/static/js/notifications.js b/static/js/notifications.js index 66182ce56e..09eb3c9d42 100644 --- a/static/js/notifications.js +++ b/static/js/notifications.js @@ -44,15 +44,6 @@ if (window.webkitNotifications) { }; } - -function browser_desktop_notifications_on() { - return notifications_api && - // Firefox on Ubuntu claims to do webkitNotifications but its notifications are terrible - /webkit/i.test(navigator.userAgent) && - // 0 is PERMISSION_ALLOWED - notifications_api.checkPermission() === 0; -} - function cancel_notification_object(notification_object) { // We must remove the .onclose so that it does not trigger on .cancel notification_object.onclose = function () {}; @@ -364,7 +355,8 @@ function process_notification(notification) { ]; } - if (notification.webkit_notify === true) { + // Firefox on Ubuntu claims to do webkitNotifications but its notifications are terrible + if (notification.desktop_notify && /webkit/i.test(navigator.userAgent)) { var icon_url = people.small_avatar_url(message); notice_memory[key] = { obj: notifications_api.createNotification(icon_url, title, content, message.id), @@ -383,7 +375,7 @@ function process_notification(notification) { delete notice_memory[key]; }; notification_object.show(); - } else if (notification.webkit_notify === false && typeof Notification !== "undefined" && /mozilla/i.test(navigator.userAgent) === true) { + } else if (notification.desktop_notify && typeof Notification !== "undefined" && /mozilla/i.test(navigator.userAgent)) { Notification.requestPermission(function (perm) { if (perm === 'granted') { notification_object = new Notification(title, { @@ -403,7 +395,7 @@ function process_notification(notification) { in_browser_notify(message, title, content, raw_operators, opts); } }); - } else if (notification.webkit_notify === false) { + } else { in_browser_notify(message, title, content, raw_operators, opts); } } @@ -537,11 +529,10 @@ exports.received_messages = function (messages) { message.notification_sent = true; if (should_send_desktop_notification(message)) { - if (browser_desktop_notifications_on()) { - process_notification({message: message, webkit_notify: true}); - } else { - process_notification({message: message, webkit_notify: false}); - } + process_notification({ + message: message, + desktop_notify: exports.granted_desktop_notifications_permission(), + }); } if (should_send_audible_notification(message) && supports_sound) { $("#notifications-area").find("audio")[0].play();