search

aiannacc / Goko-Salvager

Enhance your Dominion Online experience!
13 stars 9 forks source link

Use proper HTML5 API for all browsers #236

Closed aiannacc closed 10 years ago

aiannacc commented 10 years ago

Fixes #234. Chrome, Firefox, Safari, and Chromium all now conform to the same HTML5 Notifications API. This commit make Salvager use them all in the same way.

Also fixed the URL for the Salvager icon that was causing Kaspersky to freak out. It was still fetching it from drunkensailor.org.

I've tested both the notification creation and the notification permission requesting on:

I also haven't tested on older versions of the browsers, but they've each been properly supporting the HTML5 Notification draft spec for a while now, so we should be okay there.

This is a high-priority fix, since it's preventing opponents from joining games for everyone who uses Chrome and who has recently updated to Chrome 35. I'll deploy it (along with the rest of the current beta) as an official release as soon as someone tests and merges it.

aiannacc commented 10 years ago

Does anyone expect to have time to test this today? If not, I will just release it and hope, as at the moment, Salvager is basically just breaking Goko.

michaeljb commented 10 years ago

Tested in OS X Mavericks on:

Chrome is fine.

In Firefox, the notification appears but the text is cut off instead of wrapping to a second line -- with a guest account joining, the name was visible but the rankings were not. With a shorter name it might be visible.

Safari has the same issue as Firefox, but it looks like a bit more of the text is visible before being cut off.

The text visibility definitely seems like something minor we can worry about later. Did you want more Windows testing before merging and releasing? I have a Windows partition on my machine, but I don't really have any dev stuff set up on it so it would take me a while to get going there and I don't think I'll have time for that today.

aiannacc commented 10 years ago

The text visibility definitely seems like something minor we can worry about later. Did you want more Windows testing before merging and releasing?

Agreed. It's been cut off or wrapping for a while anyway. I'll just release it now. Thanks.