Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/notificationHelper.js

Issue 29726570: Issue 6496 - Account for browsers that don't support notifications with buttons (Closed)
Patch Set: Created March 18, 2018, 11:10 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | lib/subscriptionInit.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/notificationHelper.js
===================================================================
--- a/lib/notificationHelper.js
+++ b/lib/notificationHelper.js
@@ -206,14 +206,21 @@
// from closing automatically.
priority: 2
};
- browser.notifications.create("", notificationOptions, () =>
+
+ // Firefox and Opera don't support buttons. Firefox throws synchronously,
+ // while Opera gives an asynchronous error. Wrapping the promise like
+ // this, turns the synchronus error on Firefox into a promise rejection.
kzar 2018/03/19 08:34:59 Nit: Spelling of "synchronous"
Sebastian Noack 2018/03/19 16:58:23 Done.
+ new Promise(resolve =>
{
- // Opera does not support the addition of buttons to notifications.
- // Question type notfications always include buttons.
- if (browser.runtime.lastError && activeNotification.type != "question")
+ resolve(browser.notifications.create(notificationOptions));
+ }).catch(() =>
+ {
+ // Without buttons, showing notifications of the type "question" is
+ // pointless. For other notifications, retry with the buttons removed.
+ if (activeNotification.type != "question")
{
delete notificationOptions.buttons;
- browser.notifications.create("", notificationOptions);
+ browser.notifications.create(notificationOptions);
}
});
}
« no previous file with comments | « no previous file | lib/subscriptionInit.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld