Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
abicur opened this issue Nov 21, 2018 · 18 comments · Fixed by #1026
Closed
1 task done

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

abicur opened this issue Nov 21, 2018 · 18 comments · Fixed by #1026

Comments

@abicur
Copy link
Contributor

abicur commented Nov 21, 2018

My Setup

  • Operating System: Windows 10, Windows 8
  • App Version: 2.14.4
  • Installation type: exe
  • I have tested with the latest version
  • I can simulate the issue easily

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 Push notification causes error #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.
@abicur abicur changed the title [BUG] Windows 10 and 8 notification. Show on unread messages [BUG] Windows 10 and 8 notification. Bug with option "Show on unread messages". Nov 21, 2018
@tassoevan
Copy link
Collaborator

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

@abicur
Copy link
Contributor Author

abicur commented Nov 21, 2018

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
Copy link
Collaborator

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
Copy link
Contributor Author

abicur commented Nov 21, 2018

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 tassoevan added this to the 2.14.5 milestone Nov 21, 2018
@tassoevan
Copy link
Collaborator

tassoevan commented Nov 21, 2018

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
Copy link
Contributor Author

abicur commented Nov 21, 2018

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
Copy link
Contributor Author

abicur commented Nov 22, 2018

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
Copy link
Contributor Author

abicur commented Nov 22, 2018

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
Copy link
Contributor Author

abicur commented Nov 23, 2018

@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
Copy link
Contributor Author

abicur commented Nov 27, 2018

@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
Copy link
Collaborator

@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
Copy link
Contributor Author

abicur commented Nov 27, 2018

@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
Copy link
Collaborator

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
Copy link
Contributor Author

abicur commented Nov 29, 2018

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
Copy link
Collaborator

tassoevan commented Nov 29, 2018

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
Copy link
Contributor Author

abicur commented Nov 29, 2018

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
Copy link
Collaborator

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
Copy link
Contributor Author

abicur commented Nov 29, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants