jgrulich added a comment.
1. I'm not sure if the UI for openconnect tokens is correct, I think the QLineEdit for token secret should be on the same line, and you should probably use PasswordField instead? It can be our PasswordField widget from libs/editor/widgets/. Or it's not secret in the same sense as other secrets and it will not need to be saved by secret agent, like rest of passwords? I would also follow nm-connection-editor and make tokens options visible in the main UI, not under specific button. 2. Your code is full of trailing spaces 3. How can I try this? Is there any public Openconnect server which I can use to test this? INLINE COMMENTS > nm-openconnect-service.h:41 > #define NM_OPENCONNECT_KEY_PEM_PASSPHRASE_FSID "pem_passphrase_fsid" > +#define NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT "prevent_invalid_cert" > #define NM_OPENCONNECT_KEY_PROTOCOL "protocol" Leftover from the other review? > openconnectauth.cpp:659 > + > + const NMStringMap dataMap = d->setting->data(); > + > buttons->button(QDialogButtonBox::Ok)->setEnabled(dataMap[NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT] > != "yes"); Also leftover from the other review. > openconnectwidget.cpp:95 > + // Just popping up the tokenDlg changes nothing > + disconnect(d->ui.buTokens, &QPushButton::clicked, this, > &SettingWidget::settingChanged); > + // User cancels means nothing should change here Both disconnects can be removed if you move this all to the main widget. > openconnectwidget.cpp:199 > > d->ui.chkUseFsid->setChecked(dataMap[NM_OPENCONNECT_KEY_PEM_PASSPHRASE_FSID] > == "yes"); > + > d->ui.preventInvalidCert->setChecked(dataMap[NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT] > == "yes"); > + Leftover from the other review. > openconnectwidget.cpp:256 > data.insert(QLatin1String(NM_OPENCONNECT_KEY_PEM_PASSPHRASE_FSID), > d->ui.chkUseFsid->isChecked() ? "yes" : "no"); > + data.insert(QLatin1String(NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT), > d->ui.preventInvalidCert->isChecked() ? "yes" : "no"); > Also leftover from the other review. REVISION DETAIL https://phabricator.kde.org/D18394 To: enriquem, jgrulich Cc: pino, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
