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

Eduardo Lima (Etrunko) etrunko at redhat.com
Mon Jun 5 13:35:30 UTC 2017


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.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com




More information about the virt-tools-list mailing list