<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://reviewboard.kde.org/r/5184/">http://reviewboard.kde.org/r/5184/</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 31st, 2010, 4:23 p.m., <b>Christoph Feck</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;">Sorry, that's even worse. Now it might skip tabs because on each wheel event both the wheelDelta() as well as your new code is executed. Users will be annoyed.
I just looked at lxr.kde org, and for example [1] uses wheelDelta() signal to implement the tab switching on wheel.
We have three options:
A) Do not touch it: We respect the initial envisioned API that applications listen to that signal and use it for whatever they see fit. Not that I like that signal-based API, but just breaking it without looking at the consequences is short sighted. It probably was added so that developers don't need to put all that QT_NO_WHEELEVENT etc. stuff into their code.
B) Your initial approach: We silently discard that signal-based API by simply never emitting the signal and force developers to reimplement wheelEvent() instead when they want custom behavior. Only developers who used the signal for purposes other than tab switching will have to change their code.
C) Add a property to KTabBar that switches between A and B, with A being the default (and mention that A is deprecated, i.e. developers should set it to B, because that will be the only option in KDE 5).
Comments please.
[1] http://lxr.kde.org/source/extragear/sdk/kdevplatform-git/sublime/container.cpp#181</pre>
</blockquote>
<p>On August 31st, 2010, 5:25 p.m., <b>Thomas Lübking</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;">Personally "don't touch it" - iff touch it, maybe also reimplement KTabBar::connectNotify( "wheelDelta" ) & KTabBar::disconnectNotify( "wheelDelta" ), keep a counter and as long as it's not "0" do nothing internal on wheeling? =\</pre>
</blockquote>
<p>On August 31st, 2010, 7:30 p.m., <b>David Palacio</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 am aware there are still other applications that use KTabBar directly and too have to reimplement this basic behaviour. Even KTabWidget does it wrong when switching to disabled tabs. My proposal requires to fix applications, but then all would share the same tab switching code. Sure, certain kdelibs/app versions might misbehave but, this is the ideal solution. With enough time, it will all work fine.
A) That, indeed, is a solution. Rekonq and many other apps just have to reimplement this code. It's not a good solution imo, as KTabBar would be less capable than QTabBar is.
B) This is the less painful fix, for applications. But then the KTabBar would be lying about emitting the wheelDelta signal. I'm fine with this.
C) setWheelScrollingEnabled(bool) would be fine?
D) Find affected applications and apply necessary changes. Old versions of certain applications outside the Software Compilation would switch twice when running with newer kdelibs.
@Thomas: disconnectNotify implies connectNotify and, when the connection is made, you can not really infer what the application developer really wants. So, KTabBar::connectNotify( "wheelDelta" ) should imply setWheelScrollingEnabled(false).</pre>
</blockquote>
<p>On August 31st, 2010, 8:49 p.m., <b>Christoph Feck</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;">E) Implement the "wrapping" in Qt, and remove all "wheel on tabs" related-code from KDE and KDE applications. THAT would be the ideal solution :)
Sorry again, but we cannot do D, we just cannot break "Old versions of certain applications outside the Software Compilation [that] would switch twice when running with newer kdelibs."
Actually I like Thomas' suggestion, and we should investigate if that works. If there is no connection from wheelDelta(), then handle the scrolling in kdelibs, otherwise assume the application does it. This would avoid adding API for solution C.
Thanks for your patience so far.</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;">QObject::receivers() sounds like a deal.</pre>
<br />
<p>- Christoph</p>
<br />
<p>On August 31st, 2010, 3:54 p.m., David Palacio wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/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 kdelibs.</div>
<div>By David Palacio.</div>
<p style="color: grey;"><i>Updated 2010-08-31 15:54:54</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;">KTabWidget implements tab switching as response to mouse wheel events. This makes applications like Rekonq, that use a KTabBar instead, do nothing when the wheel is scrolled.
The proposed patch moves the behaviour implementation from KTabWidget to
KTabBar, disables and disconnects the relevant signal in KTabBar/KTabWidget.
Note that QTabBar in Qt already implements this with a small difference: it
does not wrap around the extreme tabs. That is, it will not go from the last to
the first tab and viceversa.</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;">Rekonq works as expected.
KTabWidget users, such as Konqueror and Konsole work as well.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=248962">248962</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdebase/apps/dolphin/src/dolphinmainwindow.h <span style="color: grey">(1170268)</span></li>
<li>/trunk/KDE/kdebase/apps/dolphin/src/dolphinmainwindow.cpp <span style="color: grey">(1170268)</span></li>
<li>/trunk/KDE/kdebase/apps/konsole/src/ViewContainer.cpp <span style="color: grey">(1170268)</span></li>
<li>/trunk/KDE/kdelibs/kdeui/widgets/ktabbar.cpp <span style="color: grey">(1170208)</span></li>
<li>/trunk/KDE/kdelibs/kdeui/widgets/ktabwidget.cpp <span style="color: grey">(1170208)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/5184/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>