search

RocketChat / Rocket.Chat.Electron

Official OSX, Windows, and Linux Desktop Clients for Rocket.Chat
https://rocket.chat/
MIT License
1.6k stars 703 forks source link

[BUG] Windows 10 and 8 notification. Bug with option "Show on unread messages". #1003

Closed abicur closed 5 years ago

abicur commented 5 years ago

My Setup

Description

Popup with notification is not shown. Option "Show on unread message" is not work. I can simulate it on some of our computers.

Current Behavior

  1. There is one computer with Windows 10 and another one with Windows 8. With 2.14.3 version there were error messages like at issue #979. After updating to 2.14.4 errors messages was gone, but popup with notification are not shown. If client recieved message then there is number of unread messages at tray icon, but popup is missing.
  2. Option "Show on unread messages" is not working. If client recieved message and client was hidden and "Show on unread messages" is checked then rocketchat window are not shown.

Expected Behavior

  1. Popup with notification must be shown.
  2. Rocketchat window must be shown.
tassoevan commented 5 years ago

No changes were made to "Show on unread messages". Is there any errors logged in console?

abicur commented 5 years ago

No changes were made to "Show on unread messages". Is there any errors logged in console?

May be it was broken in past? I just simulate it on my fresh installation. After I unchecked "Show on unread messages" and checked it again then I had this issue. There are no errors in the console (both "Show on unread" issue and popup notification issue)

Video with a problem

tassoevan commented 5 years ago

I'll take I look at it. BTW, I guess I didn't understand correctly the issue about notifications. In your video, the toast notification are shown normally... I guess they are the "popup" notifications you talked about?

abicur commented 5 years ago

I'll take I look at it. BTW, I guess I didn't understand correctly the issue about notifications. In your video, the toast notification are shown normally... I guess they are the "popup" notifications you talked about?

In the video is a clean installation. In it, as in most other computers, everything is fine with pop-up notifications (as in 2.13.3). But we have computers with Windows 10 and Windows 8 where this problem exists. I unfortunately can not make videos from these computers. On them, as I wrote earlier in 2.13.3 was an js error. And in the current version there is no error, but there is also no pop-up notification.

Popup with notification dont show and there are no error messages in console.

tassoevan commented 5 years ago

There is a possibility: an error is happening in the main process (the one that start the renderer ones, a.k.a. the windows) and it's being silently logged in stdout. One way to check this hypothesis is to run Rocket.Chat.exe from a console like cmd.exe or PowerShell and see what is printed there. There are plans to redirect these errors to the DevTools console in a near future.

abicur commented 5 years ago

There is a possibility: an error is happening in the main process (the one that start the renderer ones, a.k.a. the windows) and it's being silently logged in stdout. One way to check this hypothesis is to run Rocket.Chat.exe from a console like cmd.exe or PowerShell and see what is printed there. There are plans to redirect these errors to the DevTools console in a near future.

Ok, I will try it tomorrow. May be it will give more information.

abicur commented 5 years ago

I collected application logs.

In Windows 10 installation there is the same error as in version 2.14.3. The size of notification tag is too large. According to the information (windows dev center), the size should not exceed 16 characters (to support work in older systems).

Checking for update [7572:1122/082115.375:ERROR:CONSOLE(5255)] "SyntaxError: Unexpected token < in JSON at position 0", source: chrome-devto ols://devtools/bundled/inspector.js (5255) { Error: The size of notification tag is too large value at new ToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\node_modules\electron- windows-notifications\src\toast-notification.js:61:37) at WindowsToastNotification.initialize (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\ background\notifications.js:76:23) at new BaseNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\noti fications.js:10:8) at new WindowsToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\backgro und\notifications.js:68:1) at createOrGetNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\n otifications.js:184:24) at EventEmitter.events.EventEmitter.electron.ipcMain.on (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\ app.asar\app\src\background\notifications.js:199:24) at emitTwo (events.js:126:13) at EventEmitter.emit (events.js:214:7) at WebContents.<anonymous> (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\electron.asar\browser\api\web -contents.js:295:13) at emitTwo (events.js:126:13) at WebContents.emit (events.js:214:7) HRESULT: -2143420138 } { Error: The size of notification tag is too large value at new ToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\node_modules\electron- windows-notifications\src\toast-notification.js:61:37) at WindowsToastNotification.initialize (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\ background\notifications.js:76:23) at new BaseNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\noti fications.js:10:8) at new WindowsToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\backgro und\notifications.js:68:1) at createOrGetNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\n otifications.js:184:24) at EventEmitter.events.EventEmitter.electron.ipcMain.on (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\ app.asar\app\src\background\notifications.js:199:24) at emitTwo (events.js:126:13) at EventEmitter.emit (events.js:214:7) at WebContents.<anonymous> (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\electron.asar\browser\api\web -contents.js:295:13) at emitTwo (events.js:126:13) at WebContents.emit (events.js:214:7) HRESULT: -2143420138 } win10.txt

For Windows 8, the error is different and is shown in the log below. "No such interface supported" Checking for update { Error: Интерфейс не поддерживается Интерфейс не поддерживается at new ToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\node_modules\electron- windows-notifications\src\toast-notification.js:61:37) at WindowsToastNotification.initialize (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\ background\notifications.js:76:23) at new BaseNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\noti fications.js:10:8) at new WindowsToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\backgro und\notifications.js:68:1) at createOrGetNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\n otifications.js:184:24) at EventEmitter.events.EventEmitter.electron.ipcMain.on (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\ app.asar\app\src\background\notifications.js:199:24) at emitTwo (events.js:126:13) at EventEmitter.emit (events.js:214:7) at WebContents.<anonymous> (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\electron.asar\browser\api\web -contents.js:295:13) at emitTwo (events.js:126:13) at WebContents.emit (events.js:214:7) HRESULT: -2147467262 } { Error: Интерфейс не поддерживается Интерфейс не поддерживается at new ToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\node_modules\electron- windows-notifications\src\toast-notification.js:61:37) at WindowsToastNotification.initialize (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\ background\notifications.js:76:23) at new BaseNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\noti fications.js:10:8) at new WindowsToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\backgro und\notifications.js:68:1) at createOrGetNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\n otifications.js:184:24) at EventEmitter.events.EventEmitter.electron.ipcMain.on (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\ app.asar\app\src\background\notifications.js:199:24) at emitTwo (events.js:126:13) at EventEmitter.emit (events.js:214:7) at WebContents.<anonymous> (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\electron.asar\browser\api\web -contents.js:295:13) at emitTwo (events.js:126:13) at WebContents.emit (events.js:214:7) HRESULT: -2147467262 }

win8.txt

abicur commented 5 years ago

Version of Windows 10 is 1511 (build 10586.1176). Just now found one more pc with this Windows version. Problem with notifications is present.

abicur commented 5 years ago

@tassoevan In addition to issue with "Show on unread" may be it is possible to setup this option by default? It is very hard to describe to all users that the client have this possibilities and they must use it if they dont want to miss messages when client is hidden.

abicur commented 5 years ago

@tassoevan I made build version with your last commits. And then I was tested it on our computers with a problems. Errors are still present and they are the same with a past versions.

{ Error: The size of notification tag is too large value at new ToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\node_modules\electron-windows-notifications\src\toast-notification.js:61:37) at WindowsToastNotification.initialize (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\notifications.js:76:23) at new BaseNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\notifications.js:10:8) at new WindowsToastNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\notifications.js:68:1) at createOrGetNotification (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\notifications.js:184:24) at EventEmitter.events.EventEmitter.electron.ipcMain.on (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\app.asar\app\src\background\notifications.js:199:24) at emitTwo (events.js:126:13) at EventEmitter.emit (events.js:214:7) at WebContents.<anonymous> (C:\Users\user\AppData\Local\Programs\Rocket.Chat\resources\electron.asar\browser\api\web-contents.js:295:13) at emitTwo (events.js:126:13) at WebContents.emit (events.js:214:7) HRESULT: -2143420138 }

win10_2.txt

tassoevan commented 5 years ago

@abicur I haven't pushed it yet, but I applied a restriction in the tag size (which is no big deal, since the tags generated by Rocket.Chat are random 17-byte hashes). However, I still didn't make it work for Windows 8. Other popular open source message clients had problems with electron-windows-notifications and their maintainers decided to use the default Electron Notification API. My last try before do the same will be check how Slack desktop client solves this issue.

abicur commented 5 years ago

@tassoevan first of all, thank you very much for your work.

@abicur I haven't pushed it yet, but I applied a restriction in the tag size (which is no big deal, since the tags generated by Rocket.Chat are random 17-byte hashes).

As I said erly, I readed in "windows dev center" that tags length must be 16 characters for old systems (as window 10 build 1511). And I believe it would solve a problem on those systems. If you push this changes or give me your dev build then I can test it on our computers with windows 10 (build 1511).

However, I still didn't make it work for Windows 8. Other popular open source message clients had problems with electron-windows-notifications and their maintainers decided to use the default Electron Notification API. My last try before do the same will be check how Slack desktop client solves this issue.

Is it possible to use electron-windows-notifications for Windows 10(Windows 7) and use default Electron Notification API only for Windows 8?

tassoevan commented 5 years ago

Turns out that not even Slack solves the original issue that I expected to fix by the adoption of electron-windows-notifications (keep the notifications in Action Center)... I'll revert to the default Electron notifications.

abicur commented 5 years ago

Turns out that not even Slack solves the original issue that I expected to fix by the adoption of electron-windows-notifications (keep the notifications in Action Center)... I'll revert to the default Electron notifications.

@tassoevan what we will lose when default notification is used? What about notification duration? And what about issue with "show on unread"? Is it possible to setup this option by default?

tassoevan commented 5 years ago

What we will lose when default notification is used?

There is no way to customize the toast notification, which means, at this time, not having an option to reply directly on it (which is present in MacOS).

What about notification duration?

We are going to still be limited by around 20 seconds; no notifications in Action Center after that.

And what about issue with "show on unread"?

It worked normally in my tests, which is somewhat frustrating. You'd like to propose debug these lines: https://github.com/RocketChat/Rocket.Chat.Electron/blob/develop/src/scripts/events.js#L173-L182

Is it possible to setup this option by default?

Not yet.

abicur commented 5 years ago

What about notification duration?

We are going to still be limited by around 20 seconds; no notifications in Action Center after that.

So, notification duration settings will not work? It is bad news :(

And what about issue with "show on unread"?

It worked normally in my tests, which is somewhat frustrating. You'd like to propose debug these lines: https://github.com/RocketChat/Rocket.Chat.Electron/blob/develop/src/scripts/events.js#L173-L182

May be I will try to do this on at weekends.

Is it possible to setup this option by default?

Not yet.

Why is it not possible to do? I think this is a very useful option. And its benefits are even more obvious if notifications are shown for only 20 seconds. In other case it is very easy to miss a message.

tassoevan commented 5 years ago

It's not possible yet because we didn't developed a consistent way to handle these settings/preferences that might be overridden by an administrator user. I don't like the idea to add another file along with the already existing servers.json and update.json instead of unify everything into a singular settings.json file.

abicur commented 5 years ago

It's not possible yet because we didn't developed a consistent way to handle these settings/preferences that might be overridden by an administrator user. I don't like the idea to add another file along with the already existing servers.json and update.json instead of unify everything into a singular settings.json file.

Since it is not yet possible to determine these settings by the administrator (a very convenient option), is it possible to enable this option by default at least in new installations? I think this is useful and convenient. If the user does not need it, he will be able to turn it off on his own. But out of the box, this will reduce the risk of missing a message.