FF Support + Testing #64
Conversation
gauntface
force-pushed
the
ff-support-testing
branch
2 times, most recently
from
22544bc
to
579455e
Compare
| @@ -175,6 +174,9 @@ export default class PushClient extends EventDispatch { | |||
| */ | |||
| unsubscribe() { | |||
| return this.getRegistration() | |||
| .catch(() => { | |||
There was a problem hiding this comment.
I think I'd move this down to just before the final .then. If the catch happens here then we know that the next two .thens are NOOPs, so might as well not bother running them.
|
Reviewed |
|
@wibblymat just added changes from your review - thanks a lot. I think the late catching of a bad registration is adding confusion to the API here, but I've got a semi-reasonable approach for the current version. I've raised an issue to discuss if there is a better way of managing this #66 |
| // H/T to web-push for this trick to get permissions accepted / denied | ||
| initialisePromise = globalDriverReference.executeScript(() => { | ||
| /* global window, Components */ | ||
| window.netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect'); |
There was a problem hiding this comment.
Assume this is the line you were referring to that didn't have any external docs available around it. Can we link up to web-push's repo given we're referring to it a line or two above? (in case folks wonder what that project is)
gauntface
force-pushed
the
ff-support-testing
branch
from
4fcab9e
to
297e0ca
Compare
|
@wibblymat are you generally happy with this to merge in? |
|
Fwiw, LGTM for a merge. Would be good to get Mat's final call on it however |
| @@ -57,13 +58,39 @@ describe('Test Propel', function() { | |||
|
|
|||
| const queueUnitTest = browserInfo => { | |||
| it(`should pass all tests in ${browserInfo.prettyName}`, () => { | |||
|
|
|||
| switch (browserInfo.seleniumBrowserId) { | |||
There was a problem hiding this comment.
I still think that a switch with only one case is harder to read than an if
There was a problem hiding this comment.
This can be folded into the next if, in fact, right?
|
One comment on style, substance LGTM :) |
…s work and moving back to Notification API to get FF support back on the roadmap
gauntface
force-pushed
the
ff-support-testing
branch
from
5ff544d
to
c3a4144
Compare
@wibblymat
This PR brings FF back into support (Latest stable FF doesn't support permissions API),
This also uses FF for testing the with the actual API.
This has largely left the tests not too dissimilar (all the changes are hidden in the state-stub.js file).
I also changes supported() to isSupported() <- Sorry it was driving me insane.