Bug 334768 - [frameworks] Window menu in the wrong place
Summary: [frameworks] Window menu in the wrong place
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: general (show other bugs)
Version: git master
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL: https://git.reviewboard.kde.org/r/118...
Keywords:
: 334882 335720 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-05-14 15:21 UTC by Unknown
Modified: 2014-06-03 07:31 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
mgraesslin: ReviewRequest+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Unknown 2014-05-14 15:21:33 UTC
Tested only with compositing OFF on a setup with two screens.


Reproducible: Always

Steps to Reproduce:

- Open a window
- Maximize the window (this resets the bug)
- Un-maximize it so that it's movable
- Click the app menu in the topleft of the window.
- Click again to hide it.
- Move the window
- Click once again to show the app menu

The menu is in the wrong position
Comment 1 Thomas Lübking 2014-05-14 15:36:16 UTC
window menu (alt+f3) or appmenu (file/edit/...)?
Comment 2 Martin Flöser 2014-05-14 15:43:56 UTC
> window menu (alt+f3) or appmenu (file/edit/...)?

window menu - we talked on IRC and I can reproduce.
Comment 3 Martin Flöser 2014-05-15 07:36:48 UTC
relevant code in libkdecoration:

void KCommonDecoration::doShowWindowMenu()
{
    QRect menuRect = d->button[MenuButton]->rect();
    QPoint menutop = d->button[MenuButton]->mapToGlobal(menuRect.topLeft());
    QPoint menubottom = d->button[MenuButton]->mapToGlobal(menuRect.bottomRight()) + QPoint(0, 2);
    showWindowMenu(QRect(menutop, menubottom));
}

-> looks like the decoration widget doesn't get informed that it got moved.
Comment 4 Martin Flöser 2014-05-15 07:51:26 UTC
seems to be related to any configure_window call. Other way to reproduce:

1. open menu
2. send to other screen
3. open menu
Comment 5 Thomas Lübking 2014-05-15 07:59:57 UTC
Either we suck away configure events from Qt in events.cpp or mapToGlobal() is broken (QWindow related?!)
Comment 6 Martin Flöser 2014-05-15 08:35:23 UTC
interesting finding: I cannot reproduce for QWindow based decorations, only for QWidget based.
Comment 8 Martin Flöser 2014-05-15 08:50:43 UTC
On Thursday 15 May 2014 08:46:10 you wrote:
> https://bugs.kde.org/show_bug.cgi?id=334768
> 
> --- Comment #7 from Thomas Lübking <thomas.luebking@gmail.com> ---
> gutshot:
> http://quickgit.kde.org/?p=kwin.git&a=commit&h=8ecb69cd8c956e46a06c1808d3870
> 6b07a3f3a24

unlikely it just changes how we access the window id. As QWidget uses a 
QWindow internally, it's no change (yes I read the Qt code before changing 
that ;-) )
Comment 9 Martin Flöser 2014-05-16 05:57:56 UTC
> interesting finding: I cannot reproduce for QWindow based decorations, only
> for QWidget based.

might be a false trail: Aurorae uses QCursor::pos() for passing the position 
to open the menu.
Comment 10 Martin Flöser 2014-05-16 06:06:24 UTC
I have a fix, but I'm not sure whether I like it:

diff --git a/client.cpp b/client.cpp
index 1d25660..6d60cec 100644
--- a/client.cpp
+++ b/client.cpp
@@ -558,7 +558,7 @@ void Client::createDecoration(const QRect& oldgeom)
     } else if (decoration->window()) {
         decoration->window()->installEventFilter(this);
     }
-    xcb_reparent_window(connection(), decoration->window()->winId(), frameId(), 0, 0);
+    decoration->window()->setParent(QWindow::fromWinId(frameId()));
     decoration->window()->lower();
     decoration->borders(border_left, border_right, border_top, border_bottom);
     padding_left = padding_right = padding_top = padding_bottom = 0;
Comment 11 Thomas Lübking 2014-05-16 14:59:38 UTC
*** Bug 334882 has been marked as a duplicate of this bug. ***
Comment 12 Martin Flöser 2014-05-22 12:51:57 UTC
well no surprise that Qt doesn't notice that the decoration window got reparented:

    case XCB_REPARENT_NOTIFY: {
        //do not confuse Qt with these events. After all, _we_ are the
        //window manager who does the reparenting.
        return true;
    }
Comment 13 Martin Flöser 2014-05-22 12:53:49 UTC
but changing to false also doesn't fix it.
Comment 14 Thomas Lübking 2014-05-22 14:47:25 UTC
the issue is the move, not the reparenting. XCB_MOTION_NOTIFY looks more promising.
Could be due to tabbox grab bug or the altered logics.
Comment 15 Thomas Lübking 2014-05-22 16:23:44 UTC
forget what i said - it's hot.
Comment 16 Elias Probst 2014-06-02 21:43:14 UTC
*** Bug 335720 has been marked as a duplicate of this bug. ***
Comment 17 Martin Flöser 2014-06-03 07:31:11 UTC
Git commit 8543033d59c5bfb2ae034bf39fa5cf61a626d2ed by Martin Gräßlin.
Committed on 16/05/2014 at 06:12.
Pushed by graesslin into branch 'master'.

Reparent decoration window by using a QWindow wrapper for the frame

Qt doesn't like that we reparent the decoration using low level xcb
calls. So let's use a QWindow wrapper for the frame and let Qt do
the reparenting itself.
REVIEW: 118159

M  +1    -1    client.cpp
M  +2    -0    client.h
M  +1    -0    manage.cpp

http://commits.kde.org/kwin/8543033d59c5bfb2ae034bf39fa5cf61a626d2ed