<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/113821/">http://git.reviewboard.kde.org/r/113821/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 13th, 2013, 11:53 a.m. UTC, <b>David Edmundson</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/113821/diff/1/?file=213192#file213192line262" style="color: black; font-weight: bold; text-decoration: underline;">tier4/kcmutils/src/kcmultidialog.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 KCMultiDialogPrivate::init()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">260</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">KCMultiDialog</span><span class="o">::</span><span class="n">KCMultiDialog</span><span class="p">(</span><span class="n">KCMultiDialogPrivate</span> <span class="o">&</span><span class="n">dd</span><span class="p">,</span> <span class="n">KPageWidget</span> <span class="o">*</span><span class="n">pageWidget</span><span class="p">,</span> <span class="n">QWidget</span> <span class="o">*</span><span class="n">parent</span><span class="p">,</span> <span class="n">Qt</span><span class="o">::</span><span class="n">WindowFlags</span> <span class="n">flags</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">262</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">KCMultiDialog</span><span class="o">::</span><span class="n">KCMultiDialog</span><span class="p">(</span><span class="n">KCMultiDialogPrivate</span> <span class="o">&</span><span class="n">dd</span><span class="p">,</span> <span class="n">KPageWidget</span> <span class="o">*</span><span class="n">pageWidget</span><span class="p">,</span> <span class="n">QWidget</span> <span class="o">*</span><span class="n">parent</span><span class="p">,</span> <span class="n">Qt</span><span class="o">::</span><span class="n">WindowFlags</span> <span class="n">flags</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 don't think this is right.
If we copy a KCMultiDialog instance, we wouldn't copy the KPageDialog d variable of the parent object, but instead create a new one.
I think this would be a behavioural change.</pre>
</blockquote>
<p>On November 13th, 2013, 12:36 p.m. UTC, <b>Aleix Pol Gonzalez</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;">Well, if I'm not mistaken, it's the same that KPageDialog was doing already:
KPageDialog::KPageDialog(KPageDialogPrivate &dd, KPageWidget *widget, QWidget *parent, Qt::WindowFlags flags)
: QDialog(parent, flags),
d_ptr(&dd)
</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;">Previously your d pointer inherited from KPageDialog's d pointer. There was one instance of the private object, which KPageDialog copied.
Now you have two instances of private objects and two d pointers. One in KMultiDialog, one in the superclass, KPageDialog. You copy the KMultiDialogPrivate object, but the private class in KPageDialog is not copied, as we never call the copy constructor in the superclass.
</pre>
<br />
<p>- David</p>
<br />
<p>On November 12th, 2013, 6:46 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<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 and David Faure.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated Nov. 12, 2013, 6:46 p.m.</i></p>
<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 makes sure that KWidgetsAddons doesn't expose any private dependencies.
The only user of that file was KCMultiDialogPrivate. This patch refactors the code so that it's used separately.
</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;">Everything builds, couldn't test since I didn't see any test.
Tests still pass though.</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>tier1/kwidgetsaddons/src/CMakeLists.txt <span style="color: grey">(76679a4)</span></li>
<li>tier4/kcmutils/src/kcmultidialog.h <span style="color: grey">(3518736)</span></li>
<li>tier4/kcmutils/src/kcmultidialog.cpp <span style="color: grey">(9d2ccbb)</span></li>
<li>tier4/kcmutils/src/kcmultidialog_p.h <span style="color: grey">(ad5dd98)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113821/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>