<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112896/">http://git.reviewboard.kde.org/r/112896/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 26th, 2013, 2:27 a.m. UTC, <b>Fredrik Höglund</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm just going to point out something I know you already know since we've discussed it many times;
that an xcb port of the NETWM classes already exists in a branch.</pre>
</blockquote>
<p>On September 26th, 2013, 5:11 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">my aim was to write the unit test and that was the big junk of this work compared to the xcb changes (which I thought to be easier than trying to rebase the branch :-( )</pre>
</blockquote>
<p>On September 27th, 2013, 1:15 a.m. UTC, <b>Fredrik Höglund</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The problem with rebasing the branch was solved a long time ago.
There shouldn't be any conflicts except maybe in CMakeLists.txt files.
</pre>
</blockquote>
<p>On September 27th, 2013, 6:59 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">ah, I didn't know that. I will have a look and see whether I can integrate the unit tests and the bug fixes on top of your branch.</pre>
</blockquote>
<p>On October 9th, 2013, 4:45 p.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Any news about that patch?</pre>
</blockquote>
<p>On October 21st, 2013, 11:11 a.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Should I close this one too next monday?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Last warning: I'll close it next time I do my review round and there's no activity. ;-)</pre>
<br />
<p>- Kevin</p>
<br />
<p>On September 23rd, 2013, 1:11 p.m. UTC, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Sept. 23, 2013, 1:11 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is a patch series, if needed I can push the branch.
The patches address the following topics:
* add unit tests for NETRootInfo and NETWinInfo which do not require a running window manager. Test coverage of netwm.h is:
** line coverage: 75 %
** functions coverage: 84 %
** branch coverage: 62 %
The tests start their own Xvfb to have a clean state and not mess up with the Window Manager of a user or cause followint tests to fail on the CI system
* areas which are covered by unit tests are changed from XLib to XCB. This mostly affects changing and reading window properties
* API is changed from XLib types to XCB types. E.g. xcb_window_t instead of Window. Note: this is an API break, which I consider as necessary, given that especially the type "Window" is problematic.
* several bugs which were discovered through the added tests are fixed
* NETWinInfo2 is merged into NETWinInfo (marked as TODO KDE5)
* Small wrapper class for intern atom added to KXUtils. Similar code already in usage in KWin.
=====
Commits:
commit 2e50845a5d0df436106aeb776e3936691c32a753
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 23 14:31:42 2013 +0200
Use XCB for reading properties in NETRootInfo::update
Viewport handling was incorrect and is adjusted to be read correctly.
commit 23887726c03c49b4e0021c01f319653d3b9f36c5
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 23 11:41:26 2013 +0200
Use XCB for reading properties in NETWinInfo::update
Those areas which are covered by unit tests are migrated from
XGetWindowProperty to the xcb variant.
BlocksCompositing had an incorrect read - it expected a string atom,
but actually uses a cardinal. This bug is fixed.
commit 2731ebbc85eddb885700232f98e0e429cb0e066b
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 23 09:41:28 2013 +0200
Use XCB to change the Client dependent properties of NETWinInfo
Those properties for which we have unit tests are changed to XCB to
either change or delete the property.
commit 1bb85e440ec0004ef6b18b6fa1855c08c8f6697a
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Sep 20 09:58:09 2013 +0200
Unit test for Client aspect of NETWinInfo
Similar to the NETWinInfo test with WindowManager aspect, just
verifying the properties which can only be set by a Client.
The test found highlights a few possible problems:
* for some window types a fallback type is specified, but the lenght
is set to 1, thus the fallback type is not set at all
* Fullscreen monitors property is not handled in the event function
commit 2731ebbc85eddb885700232f98e0e429cb0e066b
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 23 09:41:28 2013 +0200
Use XCB to change the Client dependent properties of NETWinInfo
Those properties for which we have unit tests are changed to XCB to
either change or delete the property.
commit 1bb85e440ec0004ef6b18b6fa1855c08c8f6697a
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Sep 20 09:58:09 2013 +0200
Unit test for Client aspect of NETWinInfo
Similar to the NETWinInfo test with WindowManager aspect, just
verifying the properties which can only be set by a Client.
The test found highlights a few possible problems:
* for some window types a fallback type is specified, but the lenght
is set to 1, thus the fallback type is not set at all
* Fullscreen monitors property is not handled in the event function
commit 448f200ecdd642d1a4c85522eaa7d28072cda873
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Thu Sep 19 13:57:03 2013 +0200
Use XCB to change the WM dependent properties of NETWinInfo
Those properties for which we have unit tests are changed to XCB to
either change or delete the property.
commit 736024c6c6233a9c66a03972b9dbfbd2e6d383f4
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Thu Sep 19 13:16:03 2013 +0200
Add unit test for window manager aspect of NETWinInfo
Similar to the test for NETRootInfo it tests setting the properties
which can only be set by the window manager. Also running in its own
Xvfb.
commit 480842406c2c070280a8be8ae3e4af93674369f3
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Thu Sep 19 07:20:57 2013 +0200
Merge NETWinInfo2 into NETWinInfo
Class only existed for BIC changes and had a TODO remove KDE5 marker.
commit 4e71654d95715f5b44efef80fb95fa9bee295b6c
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Thu Sep 19 07:16:00 2013 +0200
Drop desktop argument from NETRootInfo::(set)DesktopGeometry
The desktop argument was only there because of a different semantic in
an early draft of NETwm and completely unused. Doesn't make sense to
carry it into the next version.
commit 78a818af6937a01d0464b27c8256b05e9889f692
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Wed Sep 18 14:35:52 2013 +0200
Use xcb to change properties in NETRootInfo
Change is done in all the methods which are covered by the unit test.
commit a34612f1841cf063a7e5b3e433b45e87cb16ef36
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Wed Sep 18 13:47:16 2013 +0200
NETRootInfo opperates on WindowTypeMask not on WindowType
::isSupported and ::setSupported for WindowType operated on the wrong
type. This needs to be the mask.
commit 43ebfba6ee0f309eddb9517af992e8490eef3e24
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Wed Sep 18 12:41:06 2013 +0200
Add unit test for NETRootInfo with role WindowManager
Verifies that the properties are set correctly. The unit test found
problems in the following areas:
* setSupported for window types is taking the types instead of the
mask and by that completely messing up the supported windows.
Test is set to expected fail for those elements
The unit test starts an Xvfb in the initTestCase and closes it again
when the tests end. The display number is chosen by random in [1,98].
0 is skipped as that is the normal X server and 99 is skipped as that's
what KDE's CI infrastructure already uses.
An explicit Xvfb is started instead of using an already running
X Server to not interfere with already running window managers or
left-over states from other tests.
It would be even better to have one Xvfb instance per test method,
but this is not possible as the atoms are only resolved once and
thus NETRootInfo is indirectly bound to a Display although the API
kind of implies that it would be able to use different NETRootInfo
for different Displays.
commit 99dbcd1d352c5d1fb69595899bb653ec195939fa
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 14:20:24 2013 +0200
Use xcb datatypes for helper functions in netwm.cpp
Some more Window -> xcb_window_t
commit 8a8e0edcb4c697ae8539e8a91885a0e422f30b3a
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 14:16:37 2013 +0200
Remove Xlib includes from netwm.h
We don't need them in the header any more. Only need to forward
declare the Display variable.
Unfortunately we still need the includes in other parts of
kwindowsystems where Xlib is still used and netwm.h was included.
commit e4a78dbc3fda5c20a11b6ede708f5390ac399915
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 14:06:49 2013 +0200
Use bool/true/false instead of Bool/True/False in netwm
For our internal API we can just use the proper datatypes instead of
the XLib macros.
commit 2c86f1898b8c0898a6c9c73cf90dda87b1af5095
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 13:59:48 2013 +0200
Use xcb_atom_t instead of Atom in NET(Root|Win)Info
Not all are replaced as XGetWindowProperty starts to complain if
we pass in a pointer to xcb_atom_t instead of pointer to Atom.
commit 79be0cbce1719c3fcbc0d3e89a8374192cfd6af7
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 13:50:52 2013 +0200
Use xcb_timestamp_t instead of Time in NET(Root|Win)Info
Same as with Window - using xcb type.
commit 28ae8e12894ab6cb948772bceee4ac4798417233
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 13:44:39 2013 +0200
Use xcb_window_t instead of Window in NETWinInfo
Just like for NETRootInfo.
commit c2a8a37926b16b39c0d468de0aec4cfaee0abb5d
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 13:25:09 2013 +0200
Switch NETRootInfo from Window to xcb_window_t
Window is the old XLib type whose name is rather conflicting as
it's not in a namespace. So let's use the XCB type.
For the properties which take a list of Windows, the list is
still passed as Window and not as xcb_window_t. Reason for this
is that apparently the wrong value gets written if it's the xcb
datatype. This will be removed once it is using xcb to change
the property.
commit 9ddf0634f4973a0ba4effeddca2b7edda1921e73
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 12:55:12 2013 +0200
Use KXUtils::Atom wrapper for all the items in netwm
By using the Atom wrapper the code is ported to XCB and the reply is
only retrieved once it is needed.
commit c63db861ea4224967bb0330e92ef4f914339d7bd
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 12:47:43 2013 +0200
Add xcb_connection_t* to NET(Root|Win)InfoPrivate
For future use: have an xcb_connection_t* created from the Display*
in the private classes. Once we get rid of XLib it should be passed
in through the ctor instead of the Display pointer.
commit 8b73a064a5773c3cc8b6b2207b7c3f656dbf5afb
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 12:39:18 2013 +0200
Add wrapper class for intern atom to KXUtils
Small wrapper class to simplify request and reply for an intern
atom.
commit a89950dd09a1aaf15ba196009e50aa137a490a8e
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Mon Sep 16 11:24:12 2013 +0200
Add dependency for X11_XCB to KWindowSystems framework
X11_XCB allows us to transit to XCB gradually. That is without
changing the external API.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">see unit tests, for the window manager dependent code areas I want to write some test application and include it in /test</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>tier1/kwindowsystem/autotests/CMakeLists.txt <span style="color: grey">(a1a7066)</span></li>
<li>tier1/kwindowsystem/autotests/netrootinfotestwm.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kwindowsystem/autotests/netwininfotestclient.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kwindowsystem/autotests/netwininfotestwm.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kwindowsystem/src/CMakeLists.txt <span style="color: grey">(31fb53e)</span></li>
<li>tier1/kwindowsystem/src/kstartupinfo.cpp <span style="color: grey">(971a23f)</span></li>
<li>tier1/kwindowsystem/src/kwindowinfo_x11.cpp <span style="color: grey">(f382e9c)</span></li>
<li>tier1/kwindowsystem/src/kwindowsystem_x11.cpp <span style="color: grey">(9a1f05b)</span></li>
<li>tier1/kwindowsystem/src/kxutils_p.h <span style="color: grey">(84d639b)</span></li>
<li>tier1/kwindowsystem/src/netwm.h <span style="color: grey">(71877c0)</span></li>
<li>tier1/kwindowsystem/src/netwm.cpp <span style="color: grey">(c4f9989)</span></li>
<li>tier1/kwindowsystem/src/netwm_p.h <span style="color: grey">(d7f4599)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112896/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>