<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/106041/">http://git.reviewboard.kde.org/r/106041/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 15th, 2012, 5:35 p.m., <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;">First of all: we cannot depend on anything in playground. This review has a push embargo till the library has gone through review.
Overall I dislike the extensive usage of ifdefs. I would prefer if the accessibility feature becomes a mandatory requirement for this effect to build. That is if LibKdeAccessibilityClient is not found the effect does not get build. I think this would make the code much cleaner to read and also to maintain.</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;">I wonder whether the compile time check
a) is required at all
b) a good idea
It looks to me as if the effect would through that lib just hook to some dbus signal - and one does not really have to link some lib for that purpose (that may draw in what not else, notably ati-spi) while and overmore this feature will then likely depend on the presence of some loaded kded module or so, shipped with kaccessibility, rather then the lib presence at compile time, so it actually has to check whether that is installed / loaded - yesno?</pre>
<br />
<p>- Thomas</p>
<br />
<p>On August 15th, 2012, 3:07 p.m., Amandeep Singh wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Accessibility, kwin, Frederik Gladhorn, Sebastian Sauer, and Luboš Luňák.</div>
<div>By Amandeep Singh.</div>
<p style="color: grey;"><i>Updated Aug. 15, 2012, 3:07 p.m.</i></p>
<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 patch is for KWin, makes the focus-tracking feature of KWin work with applications. This makes KDE more accessible.
A new accessibility client library "libkdeaccessibilityclient" (which can be found here: https://projects.kde.org/projects/playground/accessibility/libkdeaccessibilityclient) is also to be added in kde-support.
The focus-tracking feature will use the new library to get the accessibility information from at-spi.
Adjustments in CMakeLists have been made to add the library as an optional dependency, which if found enables the focus-tracking option in Zoom Plugin.
And in absence of library, the option is removed.</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;">I tested KWin's new feature working with KWrite and Konsole.</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>kwin/effects/CMakeLists.txt <span style="color: grey">(29accb1)</span></li>
<li>kwin/effects/zoom/CMakeLists.txt <span style="color: grey">(36e11ac)</span></li>
<li>kwin/effects/zoom/focustrackconfig.h.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/effects/zoom/zoom.h <span style="color: grey">(d809b21)</span></li>
<li>kwin/effects/zoom/zoom.cpp <span style="color: grey">(b275b1e)</span></li>
<li>kwin/effects/zoom/zoom_config.cpp <span style="color: grey">(3edd10c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106041/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>