<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/105972/">http://git.reviewboard.kde.org/r/105972/</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 13th, 2012, 2:50 p.m., <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/105972/diff/2/?file=77302#file77302line16" style="color: black; font-weight: bold; text-decoration: underline;">dolphin/src/kitemviews/kitemlistviewaccessible.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#ifndef QT_NO_ITEMVIEWS</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">On the other hand, I see lots of QModelIndex and friends here, which is something I don't particularly like. After all, the basic idea of the new view engine is to get rid of Qt's itemviews.
Is there an easy way to do it without Qt's itemviews?</pre>
</blockquote>
<p>On August 13th, 2012, 7:39 p.m., <b>Frederik Gladhorn</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;">This is a very good point. For Qt 4 there is no way to avoid QModelIndex since it's in the public API. I will look into this for Qt 5 since it's definitively something we don't want to rely on.</pre>
</blockquote>
<p>On August 14th, 2012, 10:49 a.m., <b>Frank Reininghaus</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;">It would be nice if the Qt itemviews dependency could be dropped with Qt 5.
Concerning the public API: the Qt docs don't seem to contain anything about QAccessibleTable2Interface and QAccessibleObjectEx, or am I missing something?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I just checked, Qt 5 doesn't have QModelIndex in the API any more, we actually cleaned that up.
Regarding documentation: We never officially committed to the IAccessible2 like interfaces for Qt 4, so they are marked as internal, you can find some documentation in src/gui/qaccessible2.cpp (a little in Qt 4 and some more up to date one in Qt 5). Currently our plan is to make it official in Qt 5.1. This implementation is actually very good to validate what is problematic with the APIs, thus I'm happy for the feedback.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 13th, 2012, 2:50 p.m., <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/105972/diff/2/?file=77303#file77303line67" style="color: black; font-weight: bold; text-decoration: underline;">dolphin/src/kitemviews/kitemlistviewaccessible.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int KItemListViewAccessible::rowCount() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">67</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">rowCount</span> <span class="o">=</span> <span class="n">itemCount</span> <span class="o">/</span> <span class="n">columnCount</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<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 think this will fail when grouping is enabled.</pre>
</blockquote>
<p>On August 13th, 2012, 7:39 p.m., <b>Frederik Gladhorn</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;">Frank, maybe you can give some guidance here. I didn't have time to carefully read the KItemListView etc code, but for me the row/column count/positions etc were a bit off. I wonder if there is a general problem in the code or maybe just an off-by-one somewhere.</pre>
</blockquote>
<p>On August 14th, 2012, 10:49 a.m., <b>Frank Reininghaus</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;">You're right, the rowCount issue is more tricky than it looks at first sight, at least when grouping is enabled. Just leave it as it is at the moment, I will look into that.
About the "row/column positions that are a bit off": the QAccessible::updateAccessibility() calls always add 1 to the current index, that looks strange and might be the cause of an off-by-one error. Or is there a reason why 1 has to be added?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, the +1 is one of the really ugly sides in Qt 4 accessibility. It's required, because we index children 1-based (0 is the item itself, 1 the first child). In Qt 5 this has been removed and the API is a lot more intuitive. For the time being this will have to stay this way though. I will happily do the Qt 5 port in the near future.</pre>
<br />
<p>- Frederik</p>
<br />
<p>On August 13th, 2012, 5:50 a.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 Dolphin, KDE Base Apps and KDE Accessibility.</div>
<div>By Amandeep Singh.</div>
<p style="color: grey;"><i>Updated Aug. 13, 2012, 5:50 a.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;">Added Accessibility Interfaces for Dolphin Views & Widgets, to make it accessible.
2 New files added in dolphin/ src/ kitemviews/ kitemlistviewaccessible.* that contain the three new classes.</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;">Focus-tracking tested with KMag / KWin. </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>dolphin/src/CMakeLists.txt <span style="color: grey">(5c1a6da)</span></li>
<li>dolphin/src/kitemviews/kitemlistcontainer.cpp <span style="color: grey">(5500851)</span></li>
<li>dolphin/src/kitemviews/kitemlistcontroller.cpp <span style="color: grey">(88f5d9f)</span></li>
<li>dolphin/src/kitemviews/kitemlistselectionmanager.cpp <span style="color: grey">(383914d)</span></li>
<li>dolphin/src/kitemviews/kitemlistview.h <span style="color: grey">(5723b9a)</span></li>
<li>dolphin/src/kitemviews/kitemlistview.cpp <span style="color: grey">(72b3fd8)</span></li>
<li>dolphin/src/kitemviews/kitemlistviewaccessible.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>dolphin/src/kitemviews/kitemlistviewaccessible.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>dolphin/src/kitemviews/private/kitemlistviewlayouter.h <span style="color: grey">(da5bd1d)</span></li>
<li>dolphin/src/tests/CMakeLists.txt <span style="color: grey">(3f906d1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105972/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>