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@xxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxx> 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@xxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list