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 <m...@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) <etru...@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
> etru...@redhat.com

Attachment: signature.asc
Description: PGP signature

_______________________________________________
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Reply via email to