233371 - Long tooltips should wrap instead of being cropped in gtkmozembed

Status: NEW

Description

[piers]

GtkMozEmbed version of bug 45375.

Forwarded upstream from Epiphany from:

http://bugzilla.gnome.org/show_bug.cgi?id=129456

Comment 1

[Ben Winslow]

Should this bug depend on 45375?  At the very least, it might attract a little more attention to this bug, but I've know idea whether the actual FIX is dependant on bug 45375.

Comment 2

[Sebastien Bacher]

Ubuntu bug about that: https://launchpad.net/products/firefox/+bug/37507

Comment 3

[Christian Persch (GNOME) (away; not receiving bug mail)]

Created attachment 219862 [details] [diff] [review]
possible fix

Comment 4

[Reinout van Schouwen]

Could someone be persuaded to review the patch?

Comment 5

[Alexander Sack]

Christian, you have to request review to get this in. Does the patch still apply cleanly?

Comment 6

[Ori Avtalion (salty-horse)]

The title wording is incorrect - "wrapping" is not the solution.
There are cases where the tooltip wraps but is still cropped over the edge of the screen.

Comment 7

[Alexander Sack]

Created attachment 296135 [details] [diff] [review]
updated for trunk

Comment 8

[Alexander Sack]

seems to fix the problem for me.

Comment 9

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 296135 [details] [diff] [review]
updated for trunk

I can't review my own patch.

Comment 10

[Alexander Sack]

the idea was to get feedback on whether i missed something while adapting this for trunk.

Comment 11

[Fabien Tassin]

Alexander, your patch diverged since bug 408823 landed

Comment 12

[Alexander Sack]

Created attachment 297343 [details] [diff] [review]
after bug 408823 landing

make the previous patch apply cleanly.

Comment 13

[Benjamin Smedberg]

Comment on attachment 297343 [details] [diff] [review]
after bug 408823 landing

I don't know enough GTK to review this effectively. Please try... marco, or maybe caillon would be willing.

Comment 14

[Colin Walters]

This patch looks reasonable to me.  I wonder if GTK+ 2.12's new tooltip API would be more amenable to being driven at a low level as gtkmozembed needs.

Comment 15

[timeless]

Comment on attachment 297343 [details] [diff] [review]
after bug 408823 landing

+  // Ideally we'd query the cursor size and hotspot,
+  // but there appears to be no way to do that */


odd comment syntax // */

is there an upstream bug for this?

Comment 16

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 297343 [details] [diff] [review]
after bug 408823 landing

>Index: embedding/browser/gtk/src/EmbedWindow.cpp

>+  gint x, y;
>+  gdk_window_get_origin(widget->window, &x, &y);
>+
>+  x += aXCoords;
>+  y += aYCoords;
>+
>+  // Ideally we'd query the cursor size and hotspot,
>+  // but there appears to be no way to do that */
>+  int xoffset = 8, yoffset = 8;

You're using gints above so use them here, too.  And fix the comment.


>+  if (gtk_widget_get_direction(widget) == GTK_TEXT_DIR_RTL) {
>+    x -= req.width + xoffset;
>+  } else {
>+    x += xoffset;
>+  }
> 
>-  // and show it.
>-  gtk_widget_show_all(sTipWindow);
>+  if (x + req.width > monitor.x + monitor.width)
>+    x = monitor.x + monitor.width - req.width;
>+  else if (x < monitor.x)
>+    x = monitor.x;
>+
>+  if (y + req.height + yoffset <= monitor.y + monitor.height)
>+    y += yoffset;
>+  else
>+    y -= req.height + yoffset;

Please use consistent bracing.

r=caillon with those nits addressed.

Comment 17

[Reinout van Schouwen]

Another dupe was reported in gnome bugzilla. Alexander?