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

[Victor Toso]

Hi,

On Mon, Jun 05, 2017 at 10:35:30AM -0300, Eduardo Lima (Etrunko) wrote:
> On 05/06/17 09:12, Victor Toso wrote:
> > On Fri, Jun 02, 2017 at 11:59:04AM -0300, Eduardo Lima (Etrunko) wrote:
> >> On 02/06/17 09:57, Pavel Grunt wrote:
> >>> On Fri, 2017-06-02 at 14:05 +0200, Victor Toso wrote:
> >>>> From: Victor Toso <me at victortoso.com>
> >>>>
> >>>> Mainly an issue for kiosk mode.
> >>>>
> >>>> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1446161
> >>>> Signed-off-by: Victor Toso <victortoso at redhat.com>
> >>>> ---
> >>>>  src/virt-viewer-session-spice.c | 4 ++++
> >>>>  1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-
> >>>> session-spice.c
> >>>> index 5f326aa..106abd1 100644
> >>>> --- a/src/virt-viewer-session-spice.c
> >>>> +++ b/src/virt-viewer-session-spice.c
> >>>> @@ -725,6 +725,10 @@
> >>>> virt_viewer_session_spice_main_channel_event(SpiceChannel *channel,
> >>>>          g_free(host);
> >>>>          if (!ret) {
> >>>>              g_signal_emit_by_name(session, "session-cancelled");
> >>>> +            /* ret is false when dialog did not return
> >>>> GTK_RESPONSE_OK. We
> >>>> +             * should ignore auth error dialog if user has
> >>>> cancelled or closed
> >>>> +             * the dialog */
> >>>> +            self->priv->pass_try = 0;
> >>>
> >>> I'd first change the value and after that emit the signal.
> >>>
> >>> Acked-by: Pavel Grunt <pgrunt at redhat.com>
> >>>
> >>
> >> The patch per se is 'ok-ish' for me, but I don't really understand why
> >> would we not want to show the message dialog before the authentication
> >> dialog again? I think this could be explained with more details on the
> >> commit message.
> > 
> > Sure, let me know what you think about:
> > 
> > "
> > Mainly an issue for kiosk mode due the fact that it'll not quit the
> > application on cancel. That means that authentication dialog can't
> > really be canceled and showing an input error such as "wrong password"
> > is misleading (no password should be taken in consideration on Cancel).
> > "
> > 
> 
> Yeah, much better :)
> 
> Acked-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> 
> >> Other than that, I also think the case described in the bug is trying a
> >> little bit too hard to be considered a valid use case.
> > 
> > Its still valid IMHO. Canceling a dialog should not pop-up and error
> > about user's input.
> 
> Well, I don't really see as an error, but more informative, but maybe
> just me. As for the bug report, I was talking about the case of shutting
> down and bringing the vm up back again while the dialog is running.

Ah, okay :)
Pushed as 6480e52f62bc7c87bbdca1aa50f7ff08b09e9845
> 
> -- 
> 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/20170605/77c90633/attachment.sig>