<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119337/">https://git.reviewboard.kde.org/r/119337/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 17th, 2014, 3:42 p.m. CEST, <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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">would it be possible to add a unit test for it? I know it's not easy as it depends on the window manager, but Openbox which is used on the CI system is quite decent.</p></pre>
</blockquote>
<p>On July 17th, 2014, 3:46 p.m. CEST, <b>Martin Klapetek</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I may try</p></pre>
</blockquote>
<p>On July 17th, 2014, 4:21 p.m. CEST, <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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please add/close bug 337353.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This unfortunately does not fix Dolphin :/</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've been investigating this since yesterday and the core problem of that bug seems to be in Qt, qwidget_qpa.cpp line 549 (void QWidgetPrivate::show_sys()), there's this:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> QRect geomRect <span style="color: #666666">=</span> q<span style="color: #666666">-></span>geometry();
...
<span style="color: #008000; font-weight: bold">const</span> QRect windowRect <span style="color: #666666">=</span> window<span style="color: #666666">-></span>geometry();
<span style="color: #008000; font-weight: bold">if</span> (windowRect <span style="color: #666666">!=</span> geomRect) {
<span style="color: #008000; font-weight: bold">if</span> (q<span style="color: #666666">-></span>testAttribute(Qt<span style="color: #666666">::</span>WA_Moved)
<span style="color: #666666">||</span> <span style="color: #666666">!</span>QGuiApplicationPrivate<span style="color: #666666">::</span>platformIntegration()<span style="color: #666666">-></span>hasCapability(QPlatformIntegration<span style="color: #666666">::</span>WindowManagement))
window<span style="color: #666666">-></span>setGeometry(geomRect);
<span style="color: #008000; font-weight: bold">else</span>
window<span style="color: #666666">-></span>resize(geomRect.size());
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For some reason the "if (windowRect != geomRect)" is true (where windowRect has the correctly restored size, but geomRect does not yet). Delaying the show() call in Dolphin with a single-shot timer actually makes this work properly, so my bet on that bug would be a race condition as the resize calls QXcbWindow::resize() which does async XCB calls. But I'm no expert in this so that might be a complete non-sense...</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On July 17th, 2014, 3:21 p.m. CEST, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated July 17, 2014, 3:21 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Playing a bit with the KXmlGuiWindow test showed that it didn't restore its size properly on reopening. I found out that KWindowConfig::saveWindowSize() is called before KWindowConfig::restoreWindowSize() which results in overwriting the values in the config file and KWindowConfig::restoreWindowSize() then reads the changed values.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So this patch calls KWindowConfig::saveWindowSize() only after we have applied the size from the config file. Additionally, it checks if the windowHandle() we're trying to resize is NULL as that can result in no resize and yet the d->sizeApplied being wrongly set to true.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KXmlGuiWindow (the one in tests/) now correctly stores & restores its size.</p></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>src/kmainwindow.cpp <span style="color: grey">(e273a76)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119337/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>