Signal-Desktop icon indicating copy to clipboard operation
Signal-Desktop copied to clipboard

fix #6119: respect previously maximized state when clicking tray to restore window

Open ellisonch opened this issue 1 year ago • 1 comments

First time contributor checklist:

Contributor checklist:

  • [X] My contribution is not related to translations.
  • [X] My commits are in nice logical chunks with good commit messages
  • [X] My changes are rebased on the latest main branch
  • [X] A yarn ready run passes successfully (more about tests here)
  • [x] My changes are ready to be shipped to users

Description

Fixes #6119. If the system tray is clicked, it will restore a minimized window, respecting the previous maximized state. Before this patch, it would clobber the previously maximized state.

I added an automated test that failed, and with my new changes, now passes.

Tested on Windows 10 only.

ellisonch avatar Jan 18 '24 06:01 ellisonch

Is there anything additional needed to merge this patch? The bug is still happening, and this change fixes the bug. It also includes an automated test reproducing, and verifying the change works to fix it.

ellisonch avatar Aug 25 '24 15:08 ellisonch

There is a fix in electron v35.1.4 which for me fixes this bug:

  • Fixed a bug that could cause some maximized windows on Linux to report an incorrect window state. #46464 (Also in 34, 36)

You should check again after #7263 was merged and released.

mkurz avatar Apr 08 '25 07:04 mkurz

There is a fix in electron v35.1.4 which for me fixes this bug:

* Fixed a bug that could cause some maximized windows on Linux to report an incorrect window state. [#46464](https://github.com/electron/electron/pull/46464) (Also in [34](https://github.com/electron/electron/pull/46462), [36](https://github.com/electron/electron/pull/46463))

You should check again after #7263 was merged and released.

Not fixed in 7.50.0. I don't know if that includes the patch you mentioned, nor do I know how to figure that out.

ellisonch avatar Apr 12 '25 18:04 ellisonch

7.50.0 does include the patch: https://github.com/signalapp/Signal-Desktop/blob/v7.50.0/package.json#L296

However, I can not - and also never could before - reproduce your issue. But I am on Linux anyway.

mkurz avatar Apr 12 '25 20:04 mkurz

7.50.0 does include the patch: https://github.com/signalapp/Signal-Desktop/blob/v7.50.0/package.json#L296

However, I can not - and also never could before - reproduce your issue. But I am on Linux anyway.

All I can say is the automated test that I wrote and included in this diff fails before my patch:

System tray service: rendering the tray
    1) should remember its maximized state upon restore


  0 passing (193ms)
  1 failing

  1) SystemTrayService
       should remember its maximized state upon restore:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true

      at Context.<anonymous> (ts\test-node\app\SystemTrayService_test.js:230:24)
      at process.processImmediate (node:internal/timers:476:21)

and succeeds after:

System tray service: rendering the tray
    √ should remember its maximized state upon restore (130ms)


  1 passing (183ms)

ellisonch avatar Apr 13 '25 04:04 ellisonch