Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Fabiano Fidêncio]

On Mon, Jun 20, 2016 at 5:34 PM, Marc-André Lureau
 wrote:
> Hi
>
> On Mon, Jun 20, 2016 at 5:33 PM, Marc-André Lureau
>  wrote:
>> On Mon, Jun 20, 2016 at 5:28 PM, Fabiano Fidêncio  
>> wrote:
  gtk_widget_set_can_focus(widget, true);
 +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
>>>
>>> I'd just do: s/true/TRUE before pushing, but no need to resend the
>>> patch because of this is just my preference :-)
>>
>> The line above uses lowercase 'true', I guess I'll change it too then ;)
>>
>
> Oh well, 'true' seems to win anyway:
> elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c TRUE
> src/spice-widget.c
> src/spice-widget.c:17
> elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c true
> src/spice-widget.c
> src/spice-widget.c:38
>
>
> --
> Marc-André Lureau

So, keep it as it is.
Feel free to push the patch, Marc-André.

-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Marc-André Lureau]

Hi

On Mon, Jun 20, 2016 at 5:33 PM, Marc-André Lureau
 wrote:
> On Mon, Jun 20, 2016 at 5:28 PM, Fabiano Fidêncio  
> wrote:
>>>  gtk_widget_set_can_focus(widget, true);
>>> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
>>
>> I'd just do: s/true/TRUE before pushing, but no need to resend the
>> patch because of this is just my preference :-)
>
> The line above uses lowercase 'true', I guess I'll change it too then ;)
>

Oh well, 'true' seems to win anyway:
elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c TRUE
src/spice-widget.c
src/spice-widget.c:17
elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c true
src/spice-widget.c
src/spice-widget.c:38


-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Marc-André Lureau]

On Mon, Jun 20, 2016 at 5:28 PM, Fabiano Fidêncio  wrote:
>>  gtk_widget_set_can_focus(widget, true);
>> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
>
> I'd just do: s/true/TRUE before pushing, but no need to resend the
> patch because of this is just my preference :-)

The line above uses lowercase 'true', I guess I'll change it too then ;)


-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Fabiano Fidêncio]

On Mon, Jun 20, 2016 at 3:22 PM,   wrote:
> From: Marc-André Lureau 
>
> Since the switch to a container widget (gtkstack then gtkeventbox), the
> grab may be lost when clicking on the display. Since events are treated
> at the top level container, set widget "above-child" to trap all of them
> to solve this.
>
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=96595
>
> Signed-off-by: Marc-André Lureau 
> Reported-by: Frediano Ziglio 
> ---
>  src/spice-widget.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 4ca74bf..4cfbcfa 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -632,6 +632,8 @@ static void spice_display_init(SpiceDisplay *display)
>GDK_KEY_PRESS_MASK |
>GDK_SCROLL_MASK);
>  gtk_widget_set_can_focus(widget, true);
> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);

I'd just do: s/true/TRUE before pushing, but no need to resend the
patch because of this is just my preference :-)

> +
>  d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
>  d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
>  d->mouse_cursor = get_blank_cursor();
> --
> 2.7.4
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


Acked-by: Fabiano Fidêncio 
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Marc-André Lureau]

Hi

- Original Message -
> 
> > From: Marc-André Lureau 
> > 
> > Since the switch to a container widget (gtkstack then gtkeventbox), the
> > grab may be lost when clicking on the display. Since events are treated
> > at the top level container, set widget "above-child" to trap all of them
> > to solve this.
> > 
> > Fixes:
> > https://bugs.freedesktop.org/show_bug.cgi?id=96595
> > 
> > Signed-off-by: Marc-André Lureau 
> > Reported-by: Frediano Ziglio 
> 
> I can confirm the patch fixes the issue.
> However my knowledge about Gtk+ is not enough to ack it.
> I don't know if is worth to copy the issue and how to reproduce on the
> commit message.

Imho the title is explicit enoug, but slightly more details are in the bug if 
necessary.

> 
> > ---
> >  src/spice-widget.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index 4ca74bf..4cfbcfa 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -632,6 +632,8 @@ static void spice_display_init(SpiceDisplay *display)
> >GDK_KEY_PRESS_MASK |
> >GDK_SCROLL_MASK);
> >  gtk_widget_set_can_focus(widget, true);
> > +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
> > +
> 
> TRUE, yes, would be time Gtk+ move to stdbool.h syntax.
> Or are you changing these? I can find 515 occurrences of TRUE and 68 of true.

tbh, I don't care either syntax spice-gtk has always been c99 as far as I 
remember it. You may be annoyed by the mix of uppercase/lowercase, I am not ;) 
I would rather switch to c99, and somehow enforce it with a git hook or 
something. But that's something different anyway

> 
> >  d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
> >  d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
> >  d->mouse_cursor = get_blank_cursor();
> 
> Frediano
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click

[Frediano Ziglio]

> From: Marc-André Lureau 
> 
> Since the switch to a container widget (gtkstack then gtkeventbox), the
> grab may be lost when clicking on the display. Since events are treated
> at the top level container, set widget "above-child" to trap all of them
> to solve this.
> 
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=96595
> 
> Signed-off-by: Marc-André Lureau 
> Reported-by: Frediano Ziglio 

I can confirm the patch fixes the issue.
However my knowledge about Gtk+ is not enough to ack it.
I don't know if is worth to copy the issue and how to reproduce on the
commit message.

> ---
>  src/spice-widget.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 4ca74bf..4cfbcfa 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -632,6 +632,8 @@ static void spice_display_init(SpiceDisplay *display)
>GDK_KEY_PRESS_MASK |
>GDK_SCROLL_MASK);
>  gtk_widget_set_can_focus(widget, true);
> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
> +

TRUE, yes, would be time Gtk+ move to stdbool.h syntax.
Or are you changing these? I can find 515 occurrences of TRUE and 68 of true.

>  d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
>  d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
>  d->mouse_cursor = get_blank_cursor();

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel