Re: [PATCH xwayland 3/3] Check if the frame exists before reading its size

2013-12-03 Thread Kristian Høgsberg
On Fri, Nov 15, 2013 at 11:24:03AM +0100, Axel Davy wrote:
> You were faster than me!
> 
> I was just about sending this very same patch.
> 
> The patch is correct as I was explaining in my commit message:
> "XWayland: Don't access the frame field for unmapped windows.
> 
> There are situations where weston_wm_window_get_frame_size
> and weston_wm_window_get_child_position are called on
> an unmapped window.
> In these cases, the decorations are not yet drawn,
> so we must behave as if there was no decoration.
> "
> 
> Correct the commit message ("I don't know if this correct" shouldn't be in),
> and this is
> Reviewed-by: Axel Davy 

Thanks, all three patches applied.

Kristian

> Axel Davy
> 
> On 15/11/2013 , Dima Ryazanov wrote :
> >This fixes crashes caused by popup windows that don't have override_redirect
> >(e.g., menus in VLC and KDE apps), though I don't know if this is correct.
> >
> >Signed-off-by: Dima Ryazanov 
> >---
> >  src/xwayland/window-manager.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
> >index 6d29026..eea0349 100644
> >--- a/src/xwayland/window-manager.c
> >+++ b/src/xwayland/window-manager.c
> >@@ -497,7 +497,7 @@ weston_wm_window_get_frame_size(struct weston_wm_window 
> >*window,
> > if (window->fullscreen) {
> > *width = window->width;
> > *height = window->height;
> >-} else if (window->decorate) {
> >+} else if (window->decorate && window->frame) {
> > *width = frame_width(window->frame);
> > *height = frame_height(window->frame);
> > } else {
> >@@ -515,7 +515,7 @@ weston_wm_window_get_child_position(struct 
> >weston_wm_window *window,
> > if (window->fullscreen) {
> > *x = 0;
> > *y = 0;
> >-} else if (window->decorate) {
> >+} else if (window->decorate && window->frame) {
> > frame_interior(window->frame, x, y, NULL, NULL);
> > } else {
> > *x = t->margin;
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH xwayland 3/3] Check if the frame exists before reading its size

2013-11-21 Thread Kristian Høgsberg
On Fri, Nov 15, 2013 at 02:02:23AM -0800, Dima Ryazanov wrote:
> This fixes crashes caused by popup windows that don't have override_redirect
> (e.g., menus in VLC and KDE apps), though I don't know if this is correct.
> 
> Signed-off-by: Dima Ryazanov 
> ---
>  src/xwayland/window-manager.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

First two patches looked good and are committed, but I'm not sure
about this one.  I'll take a look again tomorrow.

Kristian

> diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
> index 6d29026..eea0349 100644
> --- a/src/xwayland/window-manager.c
> +++ b/src/xwayland/window-manager.c
> @@ -497,7 +497,7 @@ weston_wm_window_get_frame_size(struct weston_wm_window 
> *window,
>   if (window->fullscreen) {
>   *width = window->width;
>   *height = window->height;
> - } else if (window->decorate) {
> + } else if (window->decorate && window->frame) {
>   *width = frame_width(window->frame);
>   *height = frame_height(window->frame);
>   } else {
> @@ -515,7 +515,7 @@ weston_wm_window_get_child_position(struct 
> weston_wm_window *window,
>   if (window->fullscreen) {
>   *x = 0;
>   *y = 0;
> - } else if (window->decorate) {
> + } else if (window->decorate && window->frame) {
>   frame_interior(window->frame, x, y, NULL, NULL);
>   } else {
>   *x = t->margin;
> -- 
> 1.8.3.2
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH xwayland 3/3] Check if the frame exists before reading its size

2013-11-15 Thread Axel Davy

You were faster than me!

I was just about sending this very same patch.

The patch is correct as I was explaining in my commit message:
"XWayland: Don't access the frame field for unmapped windows.

There are situations where weston_wm_window_get_frame_size
and weston_wm_window_get_child_position are called on
an unmapped window.
In these cases, the decorations are not yet drawn,
so we must behave as if there was no decoration.
"

Correct the commit message ("I don't know if this correct" shouldn't be in),
and this is
Reviewed-by: Axel Davy 


Axel Davy

On 15/11/2013 , Dima Ryazanov wrote :

This fixes crashes caused by popup windows that don't have override_redirect
(e.g., menus in VLC and KDE apps), though I don't know if this is correct.

Signed-off-by: Dima Ryazanov 
---
  src/xwayland/window-manager.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index 6d29026..eea0349 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -497,7 +497,7 @@ weston_wm_window_get_frame_size(struct weston_wm_window 
*window,
if (window->fullscreen) {
*width = window->width;
*height = window->height;
-   } else if (window->decorate) {
+   } else if (window->decorate && window->frame) {
*width = frame_width(window->frame);
*height = frame_height(window->frame);
} else {
@@ -515,7 +515,7 @@ weston_wm_window_get_child_position(struct weston_wm_window 
*window,
if (window->fullscreen) {
*x = 0;
*y = 0;
-   } else if (window->decorate) {
+   } else if (window->decorate && window->frame) {
frame_interior(window->frame, x, y, NULL, NULL);
} else {
*x = t->margin;


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH xwayland 3/3] Check if the frame exists before reading its size

2013-11-15 Thread Dima Ryazanov
This fixes crashes caused by popup windows that don't have override_redirect
(e.g., menus in VLC and KDE apps), though I don't know if this is correct.

Signed-off-by: Dima Ryazanov 
---
 src/xwayland/window-manager.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index 6d29026..eea0349 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -497,7 +497,7 @@ weston_wm_window_get_frame_size(struct weston_wm_window 
*window,
if (window->fullscreen) {
*width = window->width;
*height = window->height;
-   } else if (window->decorate) {
+   } else if (window->decorate && window->frame) {
*width = frame_width(window->frame);
*height = frame_height(window->frame);
} else {
@@ -515,7 +515,7 @@ weston_wm_window_get_child_position(struct weston_wm_window 
*window,
if (window->fullscreen) {
*x = 0;
*y = 0;
-   } else if (window->decorate) {
+   } else if (window->decorate && window->frame) {
frame_interior(window->frame, x, y, NULL, NULL);
} else {
*x = t->margin;
-- 
1.8.3.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel