<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/124659/">https://git.reviewboard.kde.org/r/124659/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Август 8th, 2015, 10:51 д.п. UTC, <b>David Edmundson</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;">are there any bug reports on those crashes?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you have commit access, can you push to Plasma/5.4 branch too</p></pre>
</blockquote>
<p>On Август 8th, 2015, 10:56 д.п. UTC, <b>Maxim Mikityanskiy</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 don't know if there are any bug reports, I found this bug by accident.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't have commit access, could you please push it for me?</p></pre>
</blockquote>
<p>On Август 8th, 2015, 11:16 д.п. UTC, <b>David Edmundson</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;">Is this one https://bugs.kde.org/show_bug.cgi?id=348694 ?</p></pre>
</blockquote>
<p>On Август 8th, 2015, 1:06 п.п. UTC, <b>David Edmundson</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;">Just caused the old crash, yes it is.</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;">It's difficult to say... My segfault happens because of layoutInfo == NULL inside KCMKeyboardWidget::previewLayout, so layoutInfo->variantInfos resides at bad address. Looking at stacktrace in bug report https://bugs.kde.org/show_bug.cgi?id=348694 and at Qt 5.4.1 sources, I can say that his crash happens when trying to copy invalid layoutInfo->variantInfos to QForeachContainer::c, it looks like he also has layoutInfo == NULL. But for some reason my stacktrace ends in KCMKeyboardWidgets::previewLayout, not in QList copy constructor. Maybe it's because of different compiler versions or optimization flags...</p></pre>
<br />
<p>- Maxim</p>
<br />
<p>On Август 8th, 2015, 10:39 д.п. UTC, Maxim Mikityanskiy 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 Plasma.</div>
<div>By Maxim Mikityanskiy.</div>
<p style="color: grey;"><i>Updated Авг. 8, 2015, 10:39 д.п.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</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;">There are two scenarios that lead to segfault in kcm_keyboard.</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Run kcmshell5 keyboard; switch to the second tab; in layouts table double-click on item in third column so that combo box appears; click on empty space in layouts table; [Preview] button does not get deactivated; click on that button and see kcm_keyboard crashing.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Run kcmshell5 keyboard; switch to the second tab; modify something so that [Reset] button becomes active; select any row in layouts table; click on [Reset]; row becomes deselected, but [Preview] button is still active; click on that button and see kcm_keyboard crashing.</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[Preview] button should become inactive when no rows are selected, but in these two scenarios it doesn't. KCMKeyboardWidget::layoutSelectionChanged slot does not get called in these two cases. What happens in described cases:</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">layoutsTableModel emits dataChanged signal. KCMKeyboardWidget::uiChanged slot gets called. LayoutsTableModel::refresh is called, then in QAbstractItemModel::endResetModel modelReset signal is emitted, QAbstractItemView::reset slot gets called, and it calls QItemSelectionModel::reset. QItemSelectionModel::reset disables signals and calls QItemSelectionModel::clear that calls QItemSelectionModel::clearSelection, but signals are disabled, so itemSelectionChanged is not emitted, and KCMKeyboardWidget::layoutSelectionChanged is not called.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">KCMKeyboard::load calls KCMKeyboardWidget::updateUI that calls LayoutsTableModel::refresh. All following calls are in the same order as in case 1.</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I propose to call KCMKeyboardWidget::layoutSelectionChanged manually after problematic refreshes to update buttons state. It's not the best fix, there still may be places where manual call of layoutSelectionChanged is needed, but at least it fixes two segfaults.</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>kcms/keyboard/kcm_keyboard_widget.cpp <span style="color: grey">(78ec60b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124659/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>