search

signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.49k stars 2.63k forks source link

"Update available" dialog on Windows can cause games to minimize #1753

Open haldean opened 6 years ago

haldean commented 6 years ago

Bug description

The "update available" dialog box steals focus from full-screen applications (like games) on Windows, and can cause those full-screen applications to minimize.

Steps to reproduce

Actual result: Windows notification sound plays and full screen application is minimized to show the "an update is available" dialog box, I lose my game Expected result: The full screen application remains open and keeps focus, I don't lose my game :)

Screenshots

Unfortunately, I didn't capture any debug information because I was anxious to get back to the game.

Platform info

Operating System: Windows 10 Browser: N/A (using standalone)

Signal version: whatever version came out before v1.0.37 (I'm at v1.0.37 now, but this happened before the update was applied)

Link to debug log

See previous; I didn't capture any debug information, sorry!

Dyras commented 6 years ago

You can sign me up for this! I almost had an aneurysm when it minimized Skyrim in the middle of an intense battle. From an intense dragon fight to "New update available!", talk about annoying!

Personally I think the "new update available" should appear as a message inside the app, not as a separate popup.

scienmind commented 6 years ago

IMO a good fix for that would be changing the pop-up prompt to an in-main-window bar/button that'd appear when update is ready. Similar to what browsers do (e.g. firefox)

This way it won't be disruptive, and will catch the eye in the right context, not annoying the user when he's doing something else.

(Edit: this is similar to @Dyras 's suggestion, I misunderstood it a bit the first time)

scottnonnenberg commented 6 years ago

v1.0.39 is released, with some changes to the 'new update is available' dialog. You won't see any changes in this transition (v1.0.38 -> v1.0.39), but you should when you get the next update after v1.0.39.

samureiser commented 6 years ago

I would like to report that this issue is not fixed. I just updated to v1.6.0 and the notification that there was an update still stole focus. (Even though the debug logs certainly suggest that it shouldn't have.)

INFO  2018-03-14T00:32:25.329Z Updating BrowserWindow config: {"maximized":false,"autoHideMenuBar":false,"width":794,"height":604,"x":588,"y":436}
INFO  2018-03-14T00:32:43.731Z app ready
INFO  2018-03-14T00:32:43.980Z Initializing BrowserWindow config: {"show":true,"width":794,"height":604,"minWidth":640,"minHeight":360,"autoHideMenuBar":false,"webPreferences":{"nodeIntegration":false,"preload":"C:\\{AppData}\\Local\\Programs\\signal-desktop\\resources\\app.asar\\preload.js"},"icon":"C:\\{AppData}\\Local\\Programs\\signal-desktop\\resources\\app.asar\\images\\icon_256.png","maximized":false,"x":588,"y":436}
INFO  2018-03-14T00:32:48.190Z Using OS-level spell check API with locale en_US
INFO  2018-03-14T00:32:48.677Z pre-main prep time: 4 ms
INFO  2018-03-14T00:32:48.751Z Build expires:  2018-06-11T21:54:28.000Z
INFO  2018-03-14T00:32:48.798Z background page reloaded
INFO  2018-03-14T00:32:48.798Z environment: production
INFO  2018-03-14T00:32:49.015Z ConversationController: starting initial fetch
INFO  2018-03-14T00:32:49.066Z ConversationController: done with initial fetch
INFO  2018-03-14T00:32:49.070Z New version detected: 1.6.0
INFO  2018-03-14T00:32:49.070Z listening for registration events
INFO  2018-03-14T00:32:49.074Z Rotating signed prekey...
INFO  2018-03-14T00:32:49.076Z Next signed key rotation scheduled for "2018-03-16T00:32:49.076Z"
INFO  2018-03-14T00:32:49.076Z connect
INFO  2018-03-14T00:32:49.077Z getAllFromCache
INFO  2018-03-14T00:32:49.078Z opening message socket https://textsecure-service.whispersystems.org
INFO  2018-03-14T00:32:50.260Z open inbox
INFO  2018-03-14T00:32:50.506Z getAllFromCache loaded 0 saved envelopes
INFO  2018-03-14T00:32:51.581Z websocket open
INFO  2018-03-14T00:32:51.955Z got request PUT /api/v1/queue/empty
INFO  2018-03-14T00:32:51.956Z MessageReceiver: emitting 'empty' event
INFO  2018-03-14T00:32:51.956Z Update notifications: isFocused: false isEnabled: true numNotifications: 0 shouldPlayNotificationSound: false
INFO  2018-03-14T00:32:52.252Z Saving new signed prekey 68
INFO  2018-03-14T00:32:52.252Z PUT https://textsecure-service.whispersystems.org/v2/keys/signed
INFO  2018-03-14T00:32:52.486Z PUT https://textsecure-service.whispersystems.org/v2/keys/signed 204 Success
INFO  2018-03-14T00:32:52.487Z Confirming new signed prekey 68
INFO  2018-03-14T00:32:52.545Z Most recent signed key: 68
INFO  2018-03-14T00:32:52.546Z Most recent confirmed signed key: 68
INFO  2018-03-14T00:32:52.546Z Total signed key count: 6 - 5 confirmed
INFO  2018-03-14T00:32:52.546Z Removing confirmed signed prekey: 63 with timestamp: 1520305800162
scottnonnenberg-signal commented 6 years ago

@samureiser As this doesn't appear to be happening to other people, we need more information. Please provide the details of exactly what happened, the version of windows you're running, and anything else that might have had an effect on focus/windowing on your machine.

samureiser commented 6 years ago

Interesting. If this is not happening to anybody else, then I should probably uninstall and reinstall the app in case one of the scripts got wedged somehow. I'll try that.

To answer your specific questions, this occurred while playing a game on Steam with the game in full screen mode. The update notification took focus. It's not the first time this has happened, and it has happened with other Steam games, just the first time I decided to check the issues on the repo. This occurred on Windows 8.1 Pro (x64) Version 6.3.9600 Build 9600. I'm open to the possibility that I've got some crazy configuration on my system which shouldn't but might be impacting things.

I'd say to keep this as "should be fixed" for now and I'll let you know if my remediation efforts fail and see if I can find any system-level logs that might be helpful.

Thanks!

gpaulu commented 6 years ago

I also had this happen to me today. I was playing a game in full screen mode. Signal notified me of a new version and it stole focus from the game. My PC is running Windows 10 Home (x64) Version 1709 Build 16299.309

I'm not sure exactly what version of Signal Desktop I had when this occurred, since I immediately closed Signal out of frustration when this happened, and I guess closing it caused it to update. I have 1.6.0 now.

Thanks!

scottnonnenberg-signal commented 6 years ago

Sorry, I'm not a Windows expert anymore. When other apps pop up dialogs for you to respond to, they don't break you out of your games, right? Or is it just that other apps aren't popping up dialogs?

gpaulu commented 6 years ago

I can't speak authoritatively on Windows app development, but I don't think I've ever had a pop up pull focus from a full screen game before.

samureiser commented 6 years ago

@scottnonnenberg-signal Other pop-up dialogs rarely break me out of games.

I wanted to reply and let you know that it happened again today with the update from 1.6.0 -> 1.6.1

Doing some research, this may actually be an issue with dialog.showMessageBox in Electron. Though I'm not technically savvy enough to find an open issue for it.

It might also be that the fix for #1864 in _/Signal-Desktop/app/autoupdate.js is not passing a callback (with the update notification dialog open, my Signal debug log fills up with INFO 2018-03-16T02:13:05.159Z Ignore invalid expire timer. Expected numeric 'expireTimer', got: null and the process appears to be blocked).

Far be it from me to suggest make significant changes to your codebase, but have you considered replacing dialog.showMessageBox with the HTML5 Notification API? Regardless, thanks for your help.

gasi-signal commented 6 years ago

@samureiser Thanks for providing more context.

It might also be that the fix for #1864 in /Signal-Desktop/app/auto_update.js is not passing a callback (with the update notification dialog open, my Signal debug log fills up with INFO 2018-03-16T02:13:05.159Z Ignore invalid expire timer. Expected numeric 'expireTimer', got: null and the process appears to be blocked).

I believe we need the callback to know which option the user picked: Later or Restart Signal.

Far be it from me to suggest make significant changes to your codebase, but have you considered replacing dialog.showMessageBox with the HTML5 Notification API?

That may be an option we’ll consider (I saw macOS use that for system upgrades). However, we’d have to look into what that would mean for people that have notifications disabled or operating systems where Electron notifications support is not native (Windows 7).

INFO 2018-03-16T02:13:05.159Z Ignore invalid expire timer. Expected numeric 'expireTimer', got: null

Thanks for posting this log line but I can confirm it’s not related to auto updates but rather to disappearing messages. It addresses a current behavior of our apps but should go away soon.

haldean commented 6 years ago

This just happened again on 1.7.0, and I can say that this is frustrating enough that I uninstalled Signal from my gaming PC. Here's the debug log. https://debuglogs.org/91ab735d94f0ad57b7ed834749c2519a8de46e00bccaaac60ffbc096cc89c1ed

scottnonnenberg-signal commented 6 years ago

Interesting - at least at one point, Electron dialog boxes explicitly didn't force focus of the app: https://github.com/electron/electron/issues/4719 Then again, I think that's OSX-specific. Additional searching didn't find anything.

I've moved this back to 'Bug' status and put it on our backlog. We should be able to create some quick repro projects to validate this once and for all, even if it's just a virtual machine.