Discussion:
Review Request: Add an OpenConnect VPN plug-in
Ilia Kats
2011-06-27 11:53:55 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

Review request for Network Management.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/becef66c/attachment.htm
Lamarque Vieira Souza
2011-06-27 12:37:01 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------



plasma_nm_version.h
<http://git.reviewboard.kde.org/r/101788/#comment3426>

Have you edited this manually? This file is supposed to be edited automatically but I have not sent the edit script to anyone.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3433>

Does this have a timeout?



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3429>

Add a limit to the number of messages you can append and an option to do not log anything. The plugin runs on behalf of kded, which already has plenty of problems with memory leaks.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3427>

Change this to "&Show password", which is the standard used throught all KDE programs.



vpnplugins/openconnect/openconnectwidget.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3432>

Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.



vpnplugins/openconnect/openconnectwidget.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3431>

Are you sure none of those settings can be empty? NetworkManager usually invalidates a connection if one of its settings is empty.


- Lamarque Vieira
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 11:53 a.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/fdfe97ff/attachment-0001.htm
Ilia Kats
2011-06-27 18:20:57 UTC
Permalink
Post by Ilia Kats
plasma_nm_version.h, line 3
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25381#file25381line3>
Have you edited this manually? This file is supposed to be edited automatically but I have not sent the edit script to anyone.
I am able to write my own scripts, you know. And no, I won't push it like that, but editing that part out of every version of every patch is just too much effort.
Post by Ilia Kats
vpnplugins/openconnect/openconnectauth.cpp, line 239
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25388#file25388line239>
Does this have a timeout?
All the HTTP stuff is handled by the openconnect library. I haven't dug into it's code all too deep, but I am assuming there is one.
Post by Ilia Kats
vpnplugins/openconnect/openconnectwidget.cpp, line 50
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line50>
Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.
As far as I know, this would only apply if OpenconnectAuthWidgetPrivate would inherit from SettingWidgetPrivate, which it does not. All the other VPN plugins are also deleting their d-pointers.
Post by Ilia Kats
vpnplugins/openconnect/openconnectwidget.cpp, line 79
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line79>
Are you sure none of those settings can be empty? NetworkManager usually invalidates a connection if one of its settings is empty.
Well, we definitely need a gateway to connect to, the others can probably be empty, although since these go into the VPN maps, the NM settings per se are not empty, just the sub-settings. Done in the current revision.


- Ilia


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 6:16 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/b4683f01/attachment-0001.htm
Ilia Kats
2011-06-27 18:25:04 UTC
Permalink
Post by Ilia Kats
vpnplugins/openconnect/openconnectwidget.cpp, line 50
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line50>
Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.
As far as I know, this would only apply if OpenconnectAuthWidgetPrivate would inherit from SettingWidgetPrivate, which it does not. All the other VPN plugins are also deleting their d-pointers.
OpenconnectSettingWidgetPrivate, sorry. Applies for the auth widget too, though.


- Ilia


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 6:16 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/ce0390e1/attachment.htm
Lamarque Vieira Souza
2011-06-27 18:55:13 UTC
Permalink
Post by Ilia Kats
vpnplugins/openconnect/openconnectwidget.cpp, line 50
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line50>
Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.
As far as I know, this would only apply if OpenconnectAuthWidgetPrivate would inherit from SettingWidgetPrivate, which it does not. All the other VPN plugins are also deleting their d-pointers.
OpenconnectSettingWidgetPrivate, sorry. Applies for the auth widget too, though.
So they are all double deleting. SettingWidget's destructor is virtual, if it was not then there would be no problem. Besides, SettingWidgetPrivate does not even have a destructor.
Post by Ilia Kats
plasma_nm_version.h, line 3
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25381#file25381line3>
Have you edited this manually? This file is supposed to be edited automatically but I have not sent the edit script to anyone.
I am able to write my own scripts, you know. And no, I won't push it like that, but editing that part out of every version of every patch is just too much effort.
Yeah, but this is nm09 branch, not openconnect. Or are you using a temporary branch? If so then change your script to only touch this file when running from master or nm09 branches.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 6:16 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/a155763a/attachment-0001.htm
Lamarque Vieira Souza
2011-06-27 22:28:28 UTC
Permalink
Ok, ship it.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 10:07 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/fc932a09/attachment-0001.htm
Lamarque Vieira Souza
2011-06-28 02:47:22 UTC
Permalink
Post by Ilia Kats
vpnplugins/openconnect/openconnectwidget.cpp, line 50
<http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line50>
Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.
As far as I know, this would only apply if OpenconnectAuthWidgetPrivate would inherit from SettingWidgetPrivate, which it does not. All the other VPN plugins are also deleting their d-pointers.
OpenconnectSettingWidgetPrivate, sorry. Applies for the auth widget too, though.
So they are all double deleting. SettingWidget's destructor is virtual, if it was not then there would be no problem. Besides, SettingWidgetPrivate does not even have a destructor.
Ok, I checked this and there is no double deleting because the classes that inherit from SettingWidget also declare d_ptr by theirselves. The d_ptr in them is not the same one in SettingWidget, so we really need to delete them all.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 10:07 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110628/255bef7e/attachment.htm
Ilia Kats
2011-06-27 18:16:56 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 6:16 p.m.)


Review request for Network Management.


Changes
-------

added maximum log size of 100, changed toggle password mode.
About the option not to log anything: I think that would be redundant to "view Log" from the user's point of view. Besides, the log lives in the stack, and is free'd as soon as the private class is destroyed, which happens when the dialog is destroyed, which happens a) after a successful login, or b) when the user hits "Cancel".


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
plasma_nm_version.h b580c80
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/182da740/attachment.htm
Ilia Kats
2011-06-27 19:05:10 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 7:05 p.m.)


Review request for Network Management.


Changes
-------

removed the deletes, updated the script to only modify the version if on nm09 branch.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/d9cf9d70/attachment.htm
Lamarque Vieira Souza
2011-06-27 19:25:56 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4211
-----------------------------------------------------------



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3454>

Please add some comments explaining what do they mean. For instance, name is a hostname like DNS, right? Is address an IP address? What does group mean?



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3455>

What kind of info does this return? I guess this comes from a header outside Plasma NM, if the header explains that you can just write a comment to look at the header file.

This kind of comment is usefull for someone who take over Plasma NM maintainance in the future.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3452>

remove extra line.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3456>

Connect to what host? An openconnect web server? Is that server local or in the Internet? Please write a coment explaining those questions.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3453>

inherit from widget variable, you just need to delete widget and this will be deleted too. So this line is not needed. The same is true for infoText, icon, and verticalLayout.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3457>

Does this login to what? Please write a comment explaining that.


- Lamarque Vieira
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 7:05 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/cc5f4171/attachment-0001.htm
Ilia Kats
2011-06-27 19:55:35 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 7:55 p.m.)


Review request for Network Management.


Changes
-------

Added comments, removed unnecessary deletes. I also took the liberty of copying the README file from the Gnome OpenConnect auth dialog.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/9529d2d3/attachment.htm
Lamarque Vieira Souza
2011-06-27 20:53:01 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4212
-----------------------------------------------------------



vpnplugins/openconnect/README
<http://git.reviewboard.kde.org/r/101788/#comment3458>

You can remove the parentheses.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3460>

You can change this to: "Check failed for certificate from VPN server \"%1\".\n" "Reason: %2\nAccept it anyway?"



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3459>

"from the form"



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3461>

"unsuccessful"



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3464>

Let's hope this recursive delete does not do the opposite hehe.



vpnplugins/openconnect/openconnectauth.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3462>

I think you should use deleteLater here too. All children of item will be deleted by this delete call. If item->widget() is a child of item (I guess it is) then the deleteLater call above will not be executed because item->widget will be already deleted here.


- Lamarque Vieira
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 7:55 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/d3c5d768/attachment-0001.htm
Ilia Kats
2011-06-27 21:00:46 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 9 p.m.)


Review request for Network Management.


Changes
-------

Did the changes, except for the last one. QLayoutItem doesn't have a deleteLater method and fails compilation.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/ba99241f/attachment.htm
Lamarque Vieira Souza
2011-06-27 21:28:55 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4216
-----------------------------------------------------------



vpnplugins/openconnect/openconnectwidget.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3467>

Please add a comment explaining you are setting vpn secrets to empty.


- Lamarque Vieira
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 9 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/c846a495/attachment.htm
Ilia Kats
2011-06-27 21:41:35 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 9:41 p.m.)


Review request for Network Management.


Changes
-------

added comment.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/3b09bbcb/attachment-0001.htm
Lamarque Vieira Souza
2011-06-27 22:02:34 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4218
-----------------------------------------------------------



vpnplugins/openconnect/networkmanagement_openconnectui.desktop
<http://git.reviewboard.kde.org/r/101788/#comment3471>

networkmanagement_openconnectui



vpnplugins/openconnect/nm-openconnect-service.h
<http://git.reviewboard.kde.org/r/101788/#comment3470>

Why this file is being included in the patch? It seems to be a header from openconnect's source code.



vpnplugins/openconnect/openconnectui.cpp
<http://git.reviewboard.kde.org/r/101788/#comment3468>

Move the { to the next line.


- Lamarque Vieira
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 9:41 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/667dfa2b/attachment.htm
Ilia Kats
2011-06-27 22:07:15 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------

(Updated June 27, 2011, 10:07 p.m.)


Review request for Network Management.


Changes
-------

The file is from the OpenConnect NM plugin and it's being included for the defines. We are including nm-foo-service.h with every other plug-in.


Summary
-------

Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?

This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )


This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028


Diffs (updated)
-----

libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION

Diff: http://git.reviewboard.kde.org/r/101788/diff


Testing
-------

Yes, see the bugzilla ticket.


Screenshots
-----------


http://git.reviewboard.kde.org/r/101788/s/191/


Thanks,

Ilia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/3af53f2f/attachment.htm
Commit Hook
2011-06-28 05:39:44 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4224
-----------------------------------------------------------


This review has been submitted with commit 7d066ff70da5b0a5d0564b947bcfe951fb2b19af by Ilia Kats to branch nm09.

- Commit
Post by Ilia Kats
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101788/
-----------------------------------------------------------
(Updated June 27, 2011, 10:07 p.m.)
Review request for Network Management.
Summary
-------
Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
Also, I can't figure out how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
This addresses bug 226028.
http://bugs.kde.org/show_bug.cgi?id=226028
Diffs
-----
libs/internals/settings/vpnsecrets.cpp 0d9e3f9
libs/ui/connectionsecretsjob.cpp d791bb3
libs/ui/vpnuiplugin.h 444ab2a
libs/ui/vpnuiplugin.cpp d058a52
vpnplugins/CMakeLists.txt 4706a61
vpnplugins/openconnect/CMakeLists.txt PRE-CREATION
vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION
vpnplugins/openconnect/README PRE-CREATION
vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION
vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.h PRE-CREATION
vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION
vpnplugins/openconnect/openconnectauth.ui PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION
vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION
vpnplugins/openconnect/openconnectprop.ui PRE-CREATION
vpnplugins/openconnect/openconnectui.h PRE-CREATION
vpnplugins/openconnect/openconnectui.cpp PRE-CREATION
vpnplugins/openconnect/openconnectwidget.h PRE-CREATION
vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION
Diff: http://git.reviewboard.kde.org/r/101788/diff
Testing
-------
Yes, see the bugzilla ticket.
Screenshots
-----------
http://git.reviewboard.kde.org/r/101788/s/191/
Thanks,
Ilia
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110628/73c82057/attachment.htm
Loading...