After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 785736 - textview: fix bug on DnD displaced limits of selection
textview: fix bug on DnD displaced limits of selection
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkTextView
3.22.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-08-02 14:27 UTC by Nelson Benitez
Modified: 2017-08-29 18:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
textview: fix bug on DnD displaced limits of selection (1.49 KB, patch)
2017-08-02 14:30 UTC, Nelson Benitez
none Details | Review
textview: fix bug on DnD displaced limits of selection (1.54 KB, patch)
2017-08-12 15:57 UTC, Nelson Benitez
committed Details | Review

Description Nelson Benitez 2017-08-02 14:27:48 UTC
Reproduce:
1. Configure textview to show line numbers on the left side of textview.
2. Select some text
3. Drag it (don't drop it)
4. Move it horizontally along the start (or end) of the selected text.

What should happen?
DnD icon should change from "not allowed to drop" to "permitted drop" when you move after the end iter of the selection or before the start iter of the selection.

What actually happens?
This limit is displaced by the width that the 'line numbers' (on the left of the textview) takes.
Comment 1 Nelson Benitez 2017-08-02 14:30:26 UTC
Created attachment 356783 [details] [review]
textview: fix bug on DnD displaced limits of selection

textview: fix bug on DnD displaced limits of selection
    
The fix of commit f2fd655754407103f8fb9b2c3e7586fb595ab917 should be confined to DnD coords only, because otherwise it causes the start and enf of the selection to be displaced.
Comment 2 Nelson Benitez 2017-08-02 14:35:05 UTC
CC'ing Carlos as per commit f2fd655754407103f8fb9b2c3e7586fb595ab917
Comment 3 Daniel Boles 2017-08-05 23:50:21 UTC
Review of attachment 356783 [details] [review]:

I'm not an authority, but fwiw this makes sense to me, so I see no problem if you've confirmed that it fixes the issue and doesn't cause any others.

albeit with style nitpicking as always :)

First, s/enf/end/ in the commit message.

Secondly, it would be nice to paste the whole bug URL instead of "Fixes bug NN"; that's the convention, at least.

::: gtk/gtktextview.c
@@ +9602,3 @@
 
+  priv->dnd_x = x - target_rect.x; /* DnD uses text window coords, so substract extra widget */
+  priv->dnd_y = y - target_rect.y; /* coords that happen eg. when displaying line numbers. */

I would say to instead have a single comment block above these 2 lines of code, rather than splitting the comment across them. This will also keep the line lengths down.

A multi-line comment in GTK+ style will look like this:

/* the first line
 * the last line
 */

nits: s/substract/subtract and s/eg./e.g./
Comment 4 Nelson Benitez 2017-08-12 15:57:11 UTC
Created attachment 357486 [details] [review]
textview: fix bug on DnD displaced limits of selection

Updated patch with spotted style changes. Thanks Daniel for review.
Comment 5 Daniel Boles 2017-08-28 21:38:29 UTC
Can I be a pain and ask for a minimal test case? People online seem to find

> 1. Configure textview to show line numbers on the left side of textview.

a lot more complicated than you do. :) And, e.g. GEdit already behaves fine here.
Comment 6 Nelson Benitez 2017-08-29 16:55:05 UTC
(In reply to Daniel Boles from comment #5)
> Can I be a pain and ask for a minimal test case? People online seem to find
> 
> > 1. Configure textview to show line numbers on the left side of textview.
> 
> a lot more complicated than you do. :) And, e.g. GEdit already behaves fine
> here.

It's easy, just tick the following preference:

 Gedit -> Preferences -> View -> Display line numbers

Or you can reproduce it on gnome-builder which already displays line numbers by default.
Comment 7 Daniel Boles 2017-08-29 17:44:40 UTC
Right, I had already tried GEdit as mentioned, but I misunderstood your steps, so I was checking the wrong thing. I can replicate it now. Will test the patch too. Seems like it's a no-brainer if it works.
Comment 8 Daniel Boles 2017-08-29 18:23:57 UTC
Review of attachment 357486 [details] [review]:

Makes sense, fixes it for me, and doesn't even conflict with the oddity of using Container:border-width as fixed by recent patches.
Comment 9 Daniel Boles 2017-08-29 18:28:03 UTC
Thanks for the report and patch!

Attachment 357486 [details] pushed as 0732020 - textview: fix bug on DnD displaced limits of selection