| Summary: | Quick Tile to the Top twice moves window to another screen | ||
|---|---|---|---|
| Product: | [Plasma] kwin | Reporter: | t.soernes |
| Component: | core | Assignee: | KWin default assignee <kwin-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | Flags: | mgraesslin:
Wayland+
mgraesslin: X11+ |
| Priority: | NOR | ||
| Version First Reported In: | 5.10.2 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| URL: | https://phabricator.kde.org/D6708 | ||
| Latest Commit: | https://commits.kde.org/kwin/139b4dc82a9423e42285a469ab7f7cc4139d7054 | Version Fixed/Implemented In: | 5.10.4 |
| Sentry Crash Report: | |||
| Attachments: | quick tile bottom | ||
|
Description
t.soernes
2017-07-13 18:57:36 UTC
Furthermore, quick tiling to the bottom three times (from maximized window) also produces this behavior. In my opinion, doing 'Quick Tile to the Bottom' on a window that covers the bottom half of the screen should certainly should maximize the window; if anything it should minimize it or do nothing. We don't have a quick tile to bottom Created attachment 106601 [details]
quick tile bottom
Oh the global shortcut. I thought you meant quick tile when moving the window I'm sorry, but what you consider a bug is the expected behavior. I just checked the code and found the following comment:
// If trying to tile to the side that the window is already tiled to move the window to the next
// screen if it exists, otherwise toggle the mode (set QuickTileNone)
see https://cgit.kde.org/kwin.git/tree/geometry.cpp?h=Plasma%2F5.10#n3351
I just started to add a unit test for the behavior (as that was not yet covered) and there is something fishy. For top and bottom the expected behavior should have been just moving it to the other screen and keeping the state. This had a bug. Patch at: https://phabricator.kde.org/D6708 The code you quoted is only supposed move a left-tiled window to the monitor to the left if called with a QuickTileLeft action. Similarly for right, but not top or bottom hence line 3366 ("continue; // not in horizontal line"). This could be extended to support vertical monitor setups (one atop of another) but that's not what I have and that's not what the code is currently supposed to do as far as I can tell. It should only move to another screen if there's a screen in the direction of the quicktile action, at least that's what I can glean from the code but more importantly that would be the most sensible behavior. I haven't tested your patch; perhaps what I mentioned is what you fixed.
No, what I fixed is differently. When going from top to top, the top was removed and replaced by right and left, which doesn't make sense because a window cannot be right and left at the same time. This is the maximized you experienced. I fixed the code to keep it at top, but still move to the next screen. It's a bug fix, I tried to make it not invasive. The more proper solution would be to add the vertical support as you suggest, but that would be a new feature. Git commit 139b4dc82a9423e42285a469ab7f7cc4139d7054 by Martin Flöser. Committed on 17/07/2017 at 14:51. Pushed by graesslin into branch 'Plasma/5.10'. Properly swap the quick tile side when pressing the shortcut again Summary: The quick tile shortcuts have a feature of moving the window to the next screen if one presses the same shortcut again. E.g. if a window is quick tiled to the left on the right most screen, quick tile again to left should move it to the left screen and quick tile right. Thus it swaps the borders. This wasn't handled correctly, especially not for modes like top and bottom. If a window is quick tiled to the top an impossible mode was generated: Left | Right. Doesn't make sense. This change fixes the mode swapping and extends our quick tile testing to cover the situation. FIXED-IN: 5.10.4 Test Plan: Extended test case Reviewers: #plasma, #kwin Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D6708 M +32 -18 autotests/integration/quick_tiling_test.cpp M +3 -1 geometry.cpp https://commits.kde.org/kwin/139b4dc82a9423e42285a469ab7f7cc4139d7054 |