<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/100835/">http://git.reviewboard.kde.org/r/100835/</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/100835/diff/1/?file=10903#file10903line530" style="color: black; font-weight: bold; text-decoration: underline;">applet/nmpopup.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; ">bool NMPopup::available(int state)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">481</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">Solid</span><span class="o">::</span><span class="n">Control</span><span class="o">::</span><span class="n">NetworkManager</span><span class="o">::</span><span class="n">isNetworkingEnabled</span><span class="p">())</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;">I did more changes in 591a85e2c62e10f2792517343f6fdd6316a933b6 to do almost all you have done in this review request. But I prefer to keep things simple: m_rfCheckBox and m_wwanCheckBox are visible if there is at least one interface of that type; it does not matter if Networking is enabled or not. m_rfCheckBox and m_wwanCheckBox are enabled (can be checked) if their respective hardware attribute is enabled.</pre>
</div>
<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/100835/diff/1/?file=10903#file10903line601" style="color: black; font-weight: bold; text-decoration: underline;">applet/nmpopup.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; ">bool NMPopup::available(int state)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">550</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">saveConfig</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;">I have implemented b1c517b2b42a62990b6f9dd5b96eb79659e9953b to fix bug 259375, so unless you can confirm that bug will not reappear without the readConfig/saveConfig calls I think it is better we keep readConfig/saveConfig. During my tests here even without the redundant DBus calls NM seems to disable wireless when it does to sleep, which is what happens when my notebook is suspending. But NM enables wireless even though that is not what the user wants. NM should keep wireless disabled if it was disabled by the user before the suspend.</pre>
</div>
<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/100835/diff/1/?file=10903#file10903line620" style="color: black; font-weight: bold; text-decoration: underline;">applet/nmpopup.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; ">bool NMPopup::available(int state)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">569</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_rfCheckBox</span><span class="o">-></span><span class="n">blockSignals</span><span class="p">(</span><span class="kc">true</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;">I prefer to do not send DBus calls in the "EnabledChanged" slots instead of disabling signals everytime we need to change this attribute. I really do not like to have to remember to do something before doing other thing. The real problem is not emiting the signal, the problem is calling set{Networking,Wireless,Wwan}Enabled unneedlessly in the checkboxes enabled's slots. I already fixed that in commit 591a85e2c62e10f2792517343f6fdd6316a933b6.</pre>
</div>
<br />
<p>- Lamarque Vieira</p>
<br />
<p>On March 10th, 2011, 4:10 p.m., Jirka Klimes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 Network Management.</div>
<div>By Jirka Klimes.</div>
<p style="color: grey;"><i>Updated March 10, 2011, 4:10 p.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;">This patch fixes "Enable ..." checkbox handling and simplify the code.
There was a bug that when Wifi was rfkilled by a hardware switch,
knm unchecked the "Enable wireless" checkbox, but it also (erroneously)
issued 'disable wireless' command, which caused NM to store 'wireless
disabled' state as user preference. Then after enabling hardware switch
WiFi stayed disabled.
This is now fixed and the code is also simplified a bit.
Would you review please?
For Lamarque:
I think that b1c517b2b42a62990b6f9dd5b96eb79659e9953b (saving config) is not necessary and may be contra-productive.
NM itself stores user preference and there's no need applets do that. If there are more applets they will step on their toes.
And in NM 0.9 will be perfectly possible to have more applets running.
Nonetheless, thanks for your work on knm!</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>applet/nmpopup.cpp <span style="color: grey">(282299b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100835/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>