Merge lp:~sil2100/compiz/quickfix_1141079_0.9.9 into lp:compiz/0.9.9
Status: | Merged |
---|---|
Approved by: | Łukasz Zemczak |
Approved revision: | 3645 |
Merged at revision: | 3648 |
Proposed branch: | lp:~sil2100/compiz/quickfix_1141079_0.9.9 |
Merge into: | lp:compiz/0.9.9 |
Diff against target: |
233 lines (+135/-22) 5 files modified
src/event.cpp (+21/-19) tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp (+97/-0) tests/xorg-gtest/include/compiz-xorg-gtest.h (+2/-1) tests/xorg-gtest/plugins/testhelper/src/testhelper.cpp (+1/-0) tests/xorg-gtest/src/compiz-xorg-gtest.cpp (+14/-2) |
To merge this branch: | bzr merge lp:~sil2100/compiz/quickfix_1141079_0.9.9 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Sam Spilsbury | Approve | ||
Łukasz Zemczak | Pending | ||
Daniel van Vugt | Pending | ||
Review via email: mp+156779@code.launchpad.net |
This proposal supersedes a proposal from 2013-03-14.
Commit message
Qt is stupid since it uses the stupid X11 protocol - let's not set all the attributes (especially override_redirect) every time, only in cases when it's actually needed (LP: #1141079)
Description of the change
- Problem:
The fix introduced in revision 3606 triggers a regression with Qt menus. It seems that passing on the override_redirect state from xcreatewindow results in this regression happening. Pop-up menus in Qt apps sometimes are detected as a separate window.
- Fix:
We try not to invalidly unnecessarily pass the additional attributes to the event when there is no need. This way the override_redirect attribute is not corrupted and all is cool.
- Tests:
Attached related gtest tests.
Ugh, I hate all these broken toolkits.
Now I remember - Qt was stupid and sets the override redirect bit to zero for all windows until MapRequest time, something that IMO is broken about the X protocol (you shouldn't be able to change the override redirect state as there are no events for that, and it causes race conditions later) and also broken about Qt because it actually uses that bit of the protocol.
Can you:
1. Put the wa.whatever assignments within the preceeding if () block, eg:
if (!XGetWindowAtt ributes (privateScreen.dpy, event-> xcreatewindow. window, &wa)) setDefaultWindo wAttributes (&wa); xcreatewindow. x; xcreatewindow. y; xcreatewindow. width; xcreatewindow. height; xcreatewindow. border_ width; redirect = event-> xcreatewindow. override_ redirect;
{
privateScreen.
wa.x = event->
wa.y = event->
wa.width = event->
wa.height = event->
wa.border_width = event->
wa.override_
}
I think the big concern that I had when writing this chunk of code was that destroyed windows on the server side (eg, no XGetWindowAttri butes fails) wouldn't get any attributes set, including their override_redirect attribute. For Qt windows, it means that we'll just have to work off an invalid override_redirect attribute when handling incoming restack requests relative to their now-destroyed windows, but it shouldn't be too big a deal as I think we don't restack relative to unmapped windows which haven't yet been managed. For everyone else it means we have information that should be correct in 99% of cases.
2. Move the comment up there xorg_gtest_ configure_ window. cpp for the general integration test structure XorgSystemTestW ithTestHelper fixture): tributes. override_ redirect = 1) -> Destroy Window -> Sync/Ungrab Server/Sync -> call WaitForWindowCr eation and additionally verify that the override_redirect bit is "1" WINDOW_ READY message in TestHelperWindow ctor. That would window- >overrideRedire ct () (converted to either 1 or 0) eation could return a vector of long (extracted from event.xclient. data.l) tributes. override_ redirect = 0) -> Destroy Window -> Sync/Ungrab Server/Sync -> call WaitForWindowCr eation and additionally verify that the override_redirect bit is "0" butes.override_ redirect = 0) -> call XChangeWindowAt tributes (with XWindowAttribut es.override_ redirect = 1) -> Sync/Ungrab Server/Sync -> call WaitForWindowCr eation and additionally verify that the override_redirect bit is "1"
3. Add some tests for that behavior, because this Qt "feature" is easy to forget about
- have a look in compiz_
- I think a general test for this would look like (in the AutostartCompiz
a. Grab Server -> Create Window using XCreateWindow (with XCreateWindowAt
i. testhelper.cpp should be modified to return some basic window attributes in the TEST_HELPER_
ii. WaitForWindowCr
iii. Have a simple helper function IsOverrideRedirect (std::vector <long> &data) to inspect that vector for the appropriate bit.
iv. EXPECT_TRUE that function.
b. Grab Server -> Create Window using XCreateWindow (with XCreateWindowAt
c. Grab Server -> Create Window using XCreateWindow (with XSetWindowAttri
d. Grab Server -> Create W...