[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Start building up of ClientModel with the first Client to include
From: Thomas_Lübking <thomas.luebking () web ! de>
Date: 2012-08-24 18:24:53
Message-ID: 20120824182453.12228.63034 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106139/#review17968
-----------------------------------------------------------
kwin/tabbox/clientmodel.cpp
<http://git.reviewboard.kde.org/r/106139/#comment14196>
in case c is NULL (ie. the focus chain is empty) this will crash.
I'll test whether i can crash kwin by this after having written this review (need \
to quit the browser ;-)
- Thomas Lübking
On Aug. 23, 2012, 6:47 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106139/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2012, 6:47 a.m.)
>
>
> Review request for kwin.
>
>
> Description
> -------
>
> Start building up of ClientModel with the first Client to include
>
> So far the first Client to be shown in the list (that is the
> currently active window) was inserted as the last client into
> the list by prepending it to the list.
>
> This meant that if another Client actually blocks the inclusion
> of the currently selected Client (e.g. only one window per app)
> the currently active Client never got included in this list.
>
> This change ensures that the recently used model switching has
> the starting Client as the first Client in the list and also
> simplifies the code.
>
> Stacking order switching mode is not adjusted as it seems rather
> broken, like the comment already says.
>
> Patch requires review #106088
>
>
> This addresses bug 304950.
> http://bugs.kde.org/show_bug.cgi?id=304950
>
>
> Diffs
> -----
>
> kwin/tabbox/clientmodel.cpp b78c1f5
>
> Diff: http://git.reviewboard.kde.org/r/106139/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[Attachment #5 (text/html)]
<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/106139/">http://git.reviewboard.kde.org/r/106139/</a>
</td>
</tr>
</table>
<br />
<div>
<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/106139/diff/1/?file=80264#file80264line194" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/tabbox/clientmodel.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</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; ">void \
ClientModel::createClientList(int desktop, bool partialReset)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">194</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">TabBoxClient</span><span class="o">*</span> <span class="n">stop</span> \
<span class="o">=</span> <span class="n">c</span><span class="p">;</span></pre></td> \
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">194</font></th> <td bgcolor="#ffffff" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">TabBoxClient</span><span class="o">*</span> <span class="n">stop</span> \
<span class="o">=</span> <span class="n">c</span><span class="p">;</span></pre></td> \
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">195</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k"><span \
class="hl">while</span></span><span class="hl"> </span><span class="p"><span \
class="hl">(</span></span><span class="n"><span class="hl">c</span></span><span \
class="p"><span class="hl">)</span></span> <span class="p">{</span></pre></td> <th \
bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">195</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k"><span class="hl">do</span></span> <span class="p">{</span></pre></td> \
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">196</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">QWeakPointer</span><span class="o"><</span><span \
class="n">TabBoxClient</span><span class="o">></span> <span class="n">add</span> \
<span class="o">=</span> <span class="n">tabBox</span><span \
class="o">-></span><span class="n">clientToAddToList</span><span \
class="p">(</span><span class="n">c</span><span class="p">,</span> <span \
class="n">desktop</span><span class="p">);</span></pre></td> <th bgcolor="#f0f0f0" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">196</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">QWeakPointer</span><span class="o"><</span><span \
class="n">TabBoxClient</span><span class="o">></span> <span class="n">add</span> \
<span class="o">=</span> <span class="n">tabBox</span><span \
class="o">-></span><span class="n">clientToAddToList</span><span \
class="p">(</span><span class="n">c</span><span class="p">,</span> <span \
class="n">desktop</span><span class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">in case c \
is NULL (ie. the focus chain is empty) this will crash. I'll test whether i can \
crash kwin by this after having written this review (need to quit the browser ;-)
</pre>
</div>
<br />
<p>- Thomas</p>
<br />
<p>On August 23rd, 2012, 6:47 a.m., Martin Gräßlin 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 kwin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Aug. 23, 2012, 6:47 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;">Start building up of ClientModel with the first Client to include
So far the first Client to be shown in the list (that is the
currently active window) was inserted as the last client into
the list by prepending it to the list.
This meant that if another Client actually blocks the inclusion
of the currently selected Client (e.g. only one window per app)
the currently active Client never got included in this list.
This change ensures that the recently used model switching has
the starting Client as the first Client in the list and also
simplifies the code.
Stacking order switching mode is not adjusted as it seems rather
broken, like the comment already says.
Patch requires review #106088</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="http://bugs.kde.org/show_bug.cgi?id=304950">304950</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>kwin/tabbox/clientmodel.cpp <span style="color: grey">(b78c1f5)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106139/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic