[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Olivier Fourdan  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Olivier Fourdan  changed:

   What|Removed |Added

 Attachment #343542|accepted-commit_now |committed
 status||

--- Comment #13 from Olivier Fourdan  ---
Comment on attachment 343542
  --> https://bugzilla.gnome.org/attachment.cgi?id=343542
[PATCH v3] wayland: avoid 0 width/height anchor rectangle

attachment 343542 pushed to git branch gtk-3-22 as commit 9a5ffcd - wayland:
avoid 0 width/height anchor rectangle

attachment 343542 pushed to git master as commit 4259aba - wayland: avoid 0
width/height anchor rectangle

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Jonas Ådahl  changed:

   What|Removed |Added

 Attachment #343542|none|accepted-commit_now
 status||

--- Comment #12 from Jonas Ådahl  ---
Review of attachment 343542:

Looks good to me (with one style nit)

::: gdk/wayland/gdkwindow-wayland.c
@@ +2679,3 @@
+
+  rect->width  = MAX(1, rect->width);
+  rect->height = MAX(1, rect->height);

nit: space between MAX and (

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Olivier Fourdan  changed:

   What|Removed |Added

 Attachment #343414|0   |1
is obsolete||

--- Comment #11 from Olivier Fourdan  ---
Created attachment 343542
  --> https://bugzilla.gnome.org/attachment.cgi?id=343542=edit
[PATCH v3] wayland: avoid 0 width/height anchor rectangle

v3: Make sure the anchor rectangle does not extend outside the window
geometry of the parent surface after adjusting the size and position of
the anchor rectangle to avoid a protocol error because of 0 width/height.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #10 from Jonas Ådahl  ---
(In reply to Olivier Fourdan from comment #9)
> So which one do we push, attachment 343383 [details] [review] (with a fix in
> the commit message) or attachment 343414 [details] [review] ?

I'd say we should avoid shifting the position. What if the position was at
(0,0). xdg-shell doesn't allow positioning against outside of the window. We
could do as in the latest patch, but also clamp so that its within the window
geometry.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #9 from Olivier Fourdan  ---
So which one do we push, attachment 343383 (with a fix in the commit message)
or attachment 343414 ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #8 from Olivier Fourdan  ---
(In reply to Jonas Ådahl from comment #6)
> The problem with that is that it'd be offset by 2 pixels if it flipped. Then
> again, if this is supposed to open at the "|" cursor (maybe it should still
> be a non-empty anchor that it should flip around, i.e. the width of the "|"
> cursor thing.

The flip case is not so obvious to spot to be honest, and much less likely to
occur (this is a problem that occurs only when the pango layout is empty, which
is not so common, and when the menu needs flipping, which is even less common).

I reckon that attachment 343414 is better for the most general, common case.
And tbh, I can't spot a wrong offset even when flipped using the zoom :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Olivier Fourdan  changed:

   What|Removed |Added

 Attachment #343383|accepted-commit_now |none
 status||
 Attachment #343383|0   |1
is obsolete||

--- Comment #7 from Olivier Fourdan  ---
Created attachment 343414
  --> https://bugzilla.gnome.org/attachment.cgi?id=343414=edit
[PATCH v2] wayland: avoid 0 width/height anchor rectangle

Passing a rectangle with zero width or height to xdg_shell-v6
set_anchor_rect() will cause a protocol error and terminate the client,
as with gedit when pressing the Win key.

Reason for this is because the rectangle used to set the anchor comes
from gtk_text_layout_get_iter_location() which uses the pango layout
width/height, which can be empty if there is not character at the given
location.

Make sure we don't use 0 as width or height as an anchor rectangle to
avoid the protocol error, and compensate the logical position of the
given rectangle if the size is changed, so that the actual position
remains as expected by the client.
--
v2: Move the trickery to a separate sanitize_anchor_rect() function
Compensate the position if the size is changed
Fix typo in commit message

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Jonas Ådahl  changed:

   What|Removed |Added

 CC||jad...@gmail.com

--- Comment #6 from Jonas Ådahl  ---
(In reply to Olivier Fourdan from comment #5)
> Yes, as you say, xdg-shell v6 positionner definition makes it clear that the
> rectangle should not be 0:
> 
>  
> https://cgit.freedesktop.org/wayland/wayland-protocols/tree/unstable/xdg-
> shell/xdg-shell-unstable-v6.xml#n115
> 
>   For an xdg_positioner object to be considered complete, it must have a
>   non-zero size set by set_size, and a non-zero anchor rectangle set by
>   set_anchor_rect. Passing an incomplete xdg_positioner object when
>   positioning a surface raises an error.
> 
> So the same error occurs with weston as well. 
> 
> There is a possibility that we can do, though, if the given size is zero, we
> use 1 as with attachment 343383 [details] [review]  but then compensate by
> moving the position by 1 pixel in the opposite direction.
> 
> Definitely a hack, but that would give the expected result without breaking
> the existing rules.

The problem with that is that it'd be offset by 2 pixels if it flipped. Then
again, if this is supposed to open at the "|" cursor (maybe it should still be
a non-empty anchor that it should flip around, i.e. the width of the "|" cursor
thing. So, I think the current patch is the best we can do for now, but we
should still probably allow empty-size anchor in the next xdg-positioner
version.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #5 from Olivier Fourdan  ---
Yes, as you say, xdg-shell v6 positionner definition makes it clear that the
rectangle should not be 0:

 
https://cgit.freedesktop.org/wayland/wayland-protocols/tree/unstable/xdg-shell/xdg-shell-unstable-v6.xml#n115

  For an xdg_positioner object to be considered complete, it must have a
  non-zero size set by set_size, and a non-zero anchor rectangle set by
  set_anchor_rect. Passing an incomplete xdg_positioner object when
  positioning a surface raises an error.

So the same error occurs with weston as well. 

There is a possibility that we can do, though, if the given size is zero, we
use 1 as with attachment 343383  but then compensate by moving the position by
1 pixel in the opposite direction.

Definitely a hack, but that would give the expected result without breaking the
existing rules.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #4 from Jonas Ådahl  ---
Review of attachment 343383:

Wouldn't this mean we offset the popup by 1 pixel in this case? I.e. if GTK+
wants to position something against 0x0+50+50, we'd now position it at (51,51)
if the default move-to-rect rules apply. The problem though is that I can't
think of a valid rule that positions the menu against a point|line, as the min
size of the anchor rect is as mentioned 1.

So, this particular error condition might be a mistake, and should rather be if
its negative size instead, allowing empty anchor rects.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

Matthias Clasen  changed:

   What|Removed |Added

 Attachment #343383|none|accepted-commit_now
 status||

--- Comment #3 from Matthias Clasen  ---
Review of attachment 343383:

Typo in the commit message: "rectable". In principle, the patch looks fine, but
I would suggest to reconsider if it is a wise choice to build error conditions
like this into the protocol.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #2 from Olivier Fourdan  ---
Created attachment 343383
  --> https://bugzilla.gnome.org/attachment.cgi?id=343383=edit
[PATCH] wayland: avoid 0 width/height anchor rectangle

Passing a rectable with zero width or height to xdg_shell-v6
set_anchor_rect() will cause a protocol error and terminate the client,
as with gedit when pressing the Win key.

Reason for this is because the rectangle used to set the anchor comes
from gtk_text_layout_get_iter_location() which uses the pango layout
width/height, which can be empty if there is not character at the given
location.

Make sure we don't use 0 as width or height as an anchor rectangle to
avoid the protocol error.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 777176] [wayland] gedit killed by protocol error "Invalid anchor rectangle size"

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=777176

--- Comment #1 from Olivier Fourdan  ---
gtk+ passes an invalid rect with a width of 0 to set_anchor_rect() which
triggers a Wayland protocol error.

That rect comes from pango, from:

->  popup_targets_received() in gtk+/gtk/gtktextview.c
-> gtk_text_view_get_iter_location()
-> gtk_text_layout_get_iter_location()
...
3022   pango_layout_index_to_pos (display->layout, byte_index, _rect);
3023   
3024   rect->x = PANGO_PIXELS (x_offset + pango_rect.x);
3025   rect->y += PANGO_PIXELS (pango_rect.y) + display->top_margin;
3026   rect->width = PANGO_PIXELS (pango_rect.width);
3027   rect->height = PANGO_PIXELS (pango_rect.height);

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs