*** Bug 720870 has been marked as a duplicate of this bug. ***

Created attachment 265698 [details] [review]
testtextview: Add test for gtk_text_view_add_child_in_window

As can be seen with this test its not like we were in a great shape before either. If you run this in master the child will *always* be in the specified position relative to the window no matter what you do. However, on Gtk 3.8 if you scroll with the widget visible it will follow the scroll, *but* if you force a repaint by e.g. selecting or resizing the window it will jump back to the original position wrt the window.

Indeed, i verified this with Gtk2, it has the same broken behaviour when using GTK_TEXT_WINDOW_TEXT. The only use of this seem to be gedit, so lets make it do what gedit wants.

Created attachment 265707 [details] [review]
testtextview: Add test for gtk_text_view_add_child_in_window

Created attachment 265708 [details] [review]
GtkTextView: Fix scrolling of added children

The behaviour of gtk_text_view_add_child_in_window() used to be
quite broken. It scrolled with the window during scrolling, then
jumped to the absolute position when the widget resized. Furthermore,
in 3.10 we broke the first feature, making it always be fixed.

The "proper" way to handle this is to always follow scrolling. This
is what the only user so far (gedit) wants, and if you want some
kind of overlay you should use GtkOverlay instead.

So, this changes the behaviour to something that is internally consistent
and works. I.e. all added widgets scroll with the textview as needed.

as pointed out on irc:

<nacho> alex_away, seems to be placed at the wrong place...
<nacho> alex_away, which is ok, if I need to do the window coordinates thing is ok for me too
<nacho> alex_away, also it seems to fix the other issue about drag and drop

I wonder if this is actually the right behavior... I'll need to double check though

so this is what I currently have in colorpicker:

                    start, end = bounds
                    location = self.view.get_iter_location(start)
                    win_x, win_y = self.view.buffer_to_window_coords(Gtk.TextWindowType.TEXT, location.x, location.y)
                    min_width, nat_width = self._color_button.get_preferred_width()
                    min_height, nat_height = self._color_button.get_preferred_height()
                    x = win_x
                    if win_y - nat_height > 0:
                        y = win_y - nat_height
                    else:
                        y = win_y + location.height

                    self.view.add_child_in_window(self._color_button, Gtk.TextWindowType.TEXT, x, y)

diff --git a/plugins/colorpicker/colorpicker.py b/plugins/colorpicker/colorpicker.py
index 21c27c4..7e7163f 100644
--- a/plugins/colorpicker/colorpicker.py
+++ b/plugins/colorpicker/colorpicker.py
So this fixes colorpicker and I think it makes sense.

@@ -252,14 +252,13 @@ class ColorPickerViewActivatable(GObject.Object, Gedit.ViewActivatable):
 
                     start, end = bounds
                     location = self.view.get_iter_location(start)
-                    win_x, win_y = self.view.buffer_to_window_coords(Gtk.TextWindowType.TEXT, location.x, location.y)
                     min_width, nat_width = self._color_button.get_preferred_width()
                     min_height, nat_height = self._color_button.get_preferred_height()
-                    x = win_x
-                    if win_y - nat_height > 0:
-                        y = win_y - nat_height
+                    x = location.x
+                    if location.y - nat_height > 0:
+                        y = location.y - nat_height
                     else:
-                        y = win_y + location.height
+                        y = location.y + location.height
 
                     self.view.add_child_in_window(self._color_button, Gtk.TextWindowType.TEXT, x, y)
         elif not rgba_str and self._color_button is not None:

Review of attachment 265708 [details] [review]:

maybe it is less confusing if we deprecate the old function and just mark it as broken and add a new one with the proper behavior? add_child_at_position or something like that...

Review of attachment 265707 [details] [review]:

sure. might be nice to test the other text window types while we are at it

Review of attachment 265708 [details] [review]:

I think it is fine to fix up this function to do the right thing.

Review of attachment 265708 [details] [review]:

Might be good to add a note in README.in about the change in behaviour, though

Review of attachment 265708 [details] [review]:

Might be good to add a note in README.in about the change in behaviour, though

We used to have a testtext.c app that tested all sorts of textview stuff, but it was disabled and then removed. Does anyone know why?

I think it just wasn't ported to some of the pre-3.0 removals, and maybe there was complications with doing that. If you want to try, you can bring it back

Attachment 265707 [details] pushed as 59bf558 - testtextview: Add test for gtk_text_view_add_child_in_window
Attachment 265708 [details] pushed as 664fe89 - GtkTextView: Fix scrolling of added children