<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 />
<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 Nov. 8, 2013, 4:14 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">rebased the unit tests on top of Fredrik's NETWM branch and thus I recommend to merge the NETWM branch into frameworks.
In addtion to the unit tests I added the following commits:
commit b1cc24532261ee3619f2d40de6fe1b9ab3e313ce
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 16:02:18 2013 +0100
Porting notes for NETRootInfo and NETWinInfo
commit 59fab80a735d78c093090363244dd28da6ebeb4f
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 15:51:26 2013 +0100
Pass xcb connection into ctor of NETRootInfo and NETWinInfo
With that the last usage of XLib inside netwm is gone.
Note: this is a source incompatible change.
commit 0b4be85851e22fc9265cefd78dc3046c338a368c
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 15:06:12 2013 +0100
Use xcb types instead of XLib types in netwm API (NETRootInfo, NETWinInfo)
Window -> xcb_window_t
Time -> xcb_timestamp_t
Atom -> xcb_atom_t
Inheriting classes need to adjust.
commit 239ad5baebaae942b0081b90106769758cfb3f4e
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 13:07:15 2013 +0100
Fix NETWinInfo::setVisibleName
copy'n'paste error.
commit 087d0337a70e0cba918f17fa70de67fc7149b016
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 855d1639d1c87a788773d4dd65cda51655c2e1bf
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 12:44:38 2013 +0100
Fix NETWinInfo::setBlockingCompositing
Property needs to be set on window, not on the root window.
commit 424861da0c25982f2ed9faa23f86227ba9ac0a57
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 adbc10ac322b5955aa1c11576fda1c0a4283abf4
Author: Martin Gräßlin <mgraesslin@kde.org>
Date: Fri Nov 8 12:24:05 2013 +0100
Remove NETRootInfo::screenNumber
The screen number is no longer set, so let's remove it.
commit fdaa0eea057d2b6a6511536ef59b466ae0ba5329
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 ca7f5087b4a1a1fc4deab22c66ba8f991d1594ea
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.
This will be a source incompatible change. I will take care of adjusting downstream usages (e.g. plasma, kde-workspace) when this goes in.</pre>
</td>
</tr>
</table>
<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> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>tier4/kde4support/src/kdeui/kdialog.cpp <span style="color: grey">(62c8b38)</span></li>
<li>tier2/knotifications/src/kpassivepopup.cpp <span style="color: grey">(f17086d)</span></li>
<li>tier1/kwindowsystem/src/netwm_p.h <span style="color: grey">(d7f4599)</span></li>
<li>tier1/kwindowsystem/src/netwm.cpp <span style="color: grey">(c4f9989)</span></li>
<li>tier1/kwindowsystem/src/netwm.h <span style="color: grey">(71877c0)</span></li>
<li>tier1/kwindowsystem/src/kwindowsystem_x11.cpp <span style="color: grey">(9a1f05b)</span></li>
<li>tier1/kwindowsystem/src/kwindowinfo_x11.cpp <span style="color: grey">(8a63e55)</span></li>
<li>tier1/kwindowsystem/src/kstartupinfo.cpp <span style="color: grey">(402cc97)</span></li>
<li>tier1/kwindowsystem/autotests/netwininfotestwm.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/nettesthelper.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kwindowsystem/autotests/netrootinfotestwm.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kwindowsystem/autotests/CMakeLists.txt <span style="color: grey">(da78c6f)</span></li>
<li>KDE5PORTING.html <span style="color: grey">(e888641)</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>