[virt-tools-list] [virt-viewer v2] vnc: do not show error on cancel/close of auth dialog

[Victor Toso]

Hi,

On Tue, Jun 06, 2017 at 12:03:59PM -0300, Eduardo Lima (Etrunko) wrote:
> On 06/06/17 09:12, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> >
> > Mainly a kiosk mode, similar to the spice fix in 6480e52f62b.

"Mainly a kiosk mode issue, similar ... "

> >
> > This patch saves the cancel/close state of auth dialog from
> > virt_viewer_auth_collect_credentials() in order to avoid an error
> > dialog to pop up to user in kiosk mode.
> >
> > This happens due the fact that we call virt_viewer_app_disconnected()
> > twice:
> > - One with "session-cancelled" which is correct and well handled;
> > - The other with "session-disconnected" which is misleading as there
> >   was no connection at this time. This will trigger the error dialog
> >   with "Unable to connect to the graphic server %s".
> >
>
> One of the reasons I wanted to keep it as it was in first place is
> that if we want to handle specific use cases differently, the code
> gets more and more complex over time.

Not sure if there is alternative to that.

> I did really not see anything wrong in showing the message dialog if
> authentication is canceled, but now that we have this feature in, we
> should make the behavior consistent for both SPICE and VNC.

I disagree as I mentioned a few times already. Triggering an input error
("wrong password?") while canceling an authentication dialog is
unexpected and strange behavior.

IMHO, I still think that removing the cancel button in the auth dialog
if we are in kiosk mode would be the best approach. Yeah, the code would
have a few _feature-specific_ extra bits to achieve this but we wouldn't
have a button that does nothing in the end.

But I'm not confident with my UI opinions to argument over that.

> Acked-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>

Thank you!

I'll do a small change before pushing.
The variable name should be `auth_dialog_cancelled`, cancelled with two
'l' wins in virt-viewer codebase:

$ grep -rniI "cancelled" src/*.c | wc -l
35

$ grep -rniI "canceled" src/*.c | wc -l
0

Kind regards,
    toso

>
> --
> Eduardo de Barros Lima (Etrunko)
> Software Engineer - RedHat
> etrunko at redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170607/59c846a8/attachment.sig>