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

Victor Toso victortoso at redhat.com
Mon Jun 5 12:12:33 UTC 2017


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).
"

> 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.

Cheers,
    toso
-------------- 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/49a69a89/attachment.sig>


More information about the virt-tools-list mailing list