Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2379063003:
Restore maximized window position after detaching display. (Closed)
DescriptionRestore maximized window position after detaching display.
Sometimes Windows incorrectly changes bounds of maximized windows after
attaching or detaching additional displays. In this case user can see
non-client area of the window (that should be hidden in normal case).
This workaround code restores window position if problem occurs.
BUG=651449
Committed: https://crrev.com/6bb7f81927671c8616a89ad235d797159724cf09
Cr-Commit-Position: refs/heads/master@{#424123}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Restore maximized window position after detaching display. #
Total comments: 2
Messages
Total messages: 21 (10 generated)
Description was changed from ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 ========== to ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 ==========
atimoxin@yandex-team.ru changed reviewers: + sadrul@chromium.org, sky@chromium.org
sky@chromium.org changed reviewers: + ananta@chromium.org - sadrul@chromium.org
+ananta for a second set of eyes, and -sadrul. https://codereview.chromium.org/2379063003/diff/1/ui/views/win/hwnd_message_h... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/2379063003/diff/1/ui/views/win/hwnd_message_h... ui/views/win/hwnd_message_handler.cc:1635: RECT window_rect; Would it make more sense for this to be in OnWindowPosChanging?
On 2016/09/29 18:16:23, sky wrote: > +ananta for a second set of eyes, and -sadrul. > > https://codereview.chromium.org/2379063003/diff/1/ui/views/win/hwnd_message_h... > File ui/views/win/hwnd_message_handler.cc (right): > > https://codereview.chromium.org/2379063003/diff/1/ui/views/win/hwnd_message_h... > ui/views/win/hwnd_message_handler.cc:1635: RECT window_rect; > Would it make more sense for this to be in OnWindowPosChanging? Yes, it would. I moved my hack to OnWindowPosChanging() and also found here the same code for maximized window state, but it only works if work area of the window's monitor has changed. Problem that described in bug occurs without changing monitor's work area, so I merged my hack with previously added hacks in OnWindowPosChanging().
LGTM - but wait for Ananta to review.
https://codereview.chromium.org/2379063003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/2379063003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:2233: const bool same_monitor = monitor && (monitor == last_monitor_); If a monitor is attached or detached, do we get a WM_DISPLAYCHANGE message?. If yes, then it seems better to do this part in that handler. Reason being, this function has already many edge cases which makes it a nightmare to debug problems.
https://codereview.chromium.org/2379063003/diff/20001/ui/views/win/hwnd_messa... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/2379063003/diff/20001/ui/views/win/hwnd_messa... ui/views/win/hwnd_message_handler.cc:2233: const bool same_monitor = monitor && (monitor == last_monitor_); On 2016/10/06 19:59:57, ananta wrote: > If a monitor is attached or detached, do we get a WM_DISPLAYCHANGE message?. If > yes, then it seems better to do this part in that handler. Reason being, this > function has already many edge cases which makes it a nightmare to debug > problems. Yes, we get this message, but we get it before the system tries to change window's bounds, so we can't fix bounds here. I logged related messages (using this patch: https://gist.github.com/anonymous/b62416dc29fcbb7e7ae2b7cb1a6ba9c0) and have got this result with my hack: https://gist.github.com/anonymous/9d508fe5e94652c1a9610e431a03937b and this when hack is disabled: https://gist.github.com/anonymous/9de27864177f9d4378dcb17e29f744e7
lgtm Thanks for checking.
The CQ bit was checked by atimoxin@yandex-team.ru to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by atimoxin@yandex-team.ru
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 ========== to ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 ========== to ========== Restore maximized window position after detaching display. Sometimes Windows incorrectly changes bounds of maximized windows after attaching or detaching additional displays. In this case user can see non-client area of the window (that should be hidden in normal case). This workaround code restores window position if problem occurs. BUG=651449 Committed: https://crrev.com/6bb7f81927671c8616a89ad235d797159724cf09 Cr-Commit-Position: refs/heads/master@{#424123} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/6bb7f81927671c8616a89ad235d797159724cf09 Cr-Commit-Position: refs/heads/master@{#424123}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2484643003/ by sky@chromium.org. The reason for reverting is: This resulted in a shortcut to move between monitors not working. See 656001. . |
