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 753815 - scroll fix with margin/padding
scroll fix with margin/padding
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkTextView
3.17.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 754147 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-08-19 11:56 UTC by sébastien lafargue
Modified: 2015-09-02 02:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix scroll behaviour (2.14 KB, patch)
2015-08-19 11:56 UTC, sébastien lafargue
committed Details | Review
GtkTextView: various scroll fixes (4.69 KB, patch)
2015-08-29 09:00 UTC, sébastien lafargue
none Details | Review
GtkTextView: various scroll fixes (6.14 KB, patch)
2015-09-01 17:34 UTC, sébastien lafargue
none Details | Review
GtkTextView: various scroll fixes (6.14 KB, patch)
2015-09-01 17:36 UTC, sébastien lafargue
committed Details | Review

Description sébastien lafargue 2015-08-19 11:56:20 UTC
Created attachment 309566 [details] [review]
fix scroll behaviour

expected behaviour:

When you move line by line, only padding is
automatically shown and you need to use Page key to show margin.

This fix cursor going out of the screen bug too.

There still some pixelcache prob, i need to check Christian for this.

this fix is a follow up to :

https://bugzilla.gnome.org/show_bug.cgi?id=406159
Comment 1 Matthias Clasen 2015-08-19 20:09:02 UTC
Review of attachment 309566 [details] [review]:

ok
Comment 2 sébastien lafargue 2015-08-19 20:54:05 UTC
Review of attachment 309566 [details] [review]:

commited as 8baab8f
Comment 3 sébastien lafargue 2015-08-19 20:55:13 UTC
I keep the report opened to fix the scroll prob
Comment 4 sébastien lafargue 2015-08-26 23:04:27 UTC
*** Bug 754147 has been marked as a duplicate of this bug. ***
Comment 5 sébastien lafargue 2015-08-29 09:00:49 UTC
Created attachment 310237 [details] [review]
GtkTextView: various scroll fixes

Due to the introduction of view's margin and padding,
some bugs in scrolling behaviour have come.
This commit fix them.
Comment 6 Adam Williamson 2015-08-31 22:40:17 UTC
I tested a GTK+ built with both slaf's patches (the one already merged, and the one from #c5) and it looks OK for me (solves the issue with gedit jumping around a lot).
Comment 7 Matthias Clasen 2015-09-01 04:47:07 UTC
Review of attachment 310237 [details] [review]:

I haven't tested this in detail, mainly because the commit message doesn't actually describe the bugs in enough detail to verify. It would be good if it described both the scrolling behaviour you are trying to achieve here, and in what way the code was failing to implement it.
Comment 8 Matthias Clasen 2015-09-01 04:47:07 UTC
Review of attachment 310237 [details] [review]:

I haven't tested this in detail, mainly because the commit message doesn't actually describe the bugs in enough detail to verify. It would be good if it described both the scrolling behaviour you are trying to achieve here, and in what way the code was failing to implement it.
Comment 9 Adam Williamson 2015-09-01 05:02:50 UTC
mclasen: the code as merged has a clear and annoying bug:

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

which causes gedit to keep jumping the line you're editing to the bottom of the window. This fixes that, at least.
Comment 10 Matthias Clasen 2015-09-01 05:03:41 UTC
Attachment 310237 [details] pushed as 016f659 - GtkTextView: various scroll fixes
Comment 11 Matthias Clasen 2015-09-01 05:06:05 UTC
sorry, didn't mean to push this yet
Comment 12 Matthias Clasen 2015-09-01 05:07:16 UTC
if you add some of the symptom descriptions from that gedit bug in the commit message, that would be great
Comment 13 sébastien lafargue 2015-09-01 08:53:56 UTC
hi,

Here is a list of fixed symptom seen before this patch:

Padding not displayed correctly when moving cursor at beginning/end of lines

One pixel shift in x or y in some rendered bottom text ( fixed by converting adjustment float values to integer before the calculation )

Some horizontal jumps forward and backward when using Page right key at the end of the longest line.

retval return value of _gtk_text_view_scroll_to_iter wrong in some cases
Comment 14 sébastien lafargue 2015-09-01 10:47:42 UTC
i'll update the commit message of the patch ASAP and will try to be as precise as possible
Comment 15 sébastien lafargue 2015-09-01 17:34:20 UTC
Created attachment 310438 [details] [review]
GtkTextView: various scroll fixes

The purpose of this patch is to fix regressions in GtkTextView
scroll behaviours due to commit d138156.
( addition of padding and margins to the view )

Adding some padding is done by, for example, in inspector css tab with:

GtkTextView {
  padding: 10px 10px 10px 10px;
}

and adding margins, by changing one of *-margin properties
( * standing for left/right/top/bottom ) or the corresponding
accessor functions.

Understand that none of these bugs are easy to trigger.
What's happened is that a old and wrong version of the code of the code
( lost in the mean time ) was pushed.

These bugs are best seen with wrap mode set to off.

The commit 8baab8f fix a first regression.

This one is about:

- Cursor going out of the view at line ends instead of being visible
  or triggering the horizontal scroll.

- Padding not displayed correctly
  when moving cursor at beginning/end of lines

- When horizontal scroll position not at left, cursor can make scroll
  by more than one character (you need left padding to see this )

- Moving the cursor arround, the rendered text can be shitted in x or y.
  ( fixed by converting adjustment float values
  to integer before calculations )

  It can be observed by going down with the cursor more
  than the view height then going up

- retval return value of _gtk_text_view_scroll_to_iter wrong in some cases

In addition, this patch re-factor priv->top_border
in screen_dest.y calculation

Of course, all GtkTextView and GtkSourceView based app were impacted
by these bugs ( gedit for example, see bug 754147 )

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

https://bugzilla.gnome.org/show_bug.cgi?id=75815
Comment 16 sébastien lafargue 2015-09-01 17:36:42 UTC
Created attachment 310439 [details] [review]
GtkTextView: various scroll fixes

The purpose of this patch is to fix regressions in GtkTextView
scroll behaviours due to commit d138156.
( addition of padding and margins to the view )

Adding some padding is done by, for example, in inspector css tab with:

GtkTextView {
  padding: 10px 10px 10px 10px;
}

and adding margins, by changing one of *-margin properties
( * standing for left/right/top/bottom ) or the corresponding
accessor functions.

Understand that none of these bugs are easy to trigger.
What's happened is that a old and wrong version of the code of the code
( lost in the mean time ) was pushed.

These bugs are best seen with wrap mode set to off.

The commit 8baab8f fix a first regression.

This one is about:

- Cursor going out of the view at line ends instead of being visible
  or triggering the horizontal scroll.

- Padding not displayed correctly
  when moving cursor at beginning/end of lines

- When horizontal scroll position not at left, cursor can make scroll
  by more than one character (you need left padding to see this )

- Moving the cursor arround, the rendered text can be shitted in x or y.
  ( fixed by converting adjustment float values
  to integer before calculations )

  It can be observed by going down with the cursor more
  than the view height then going up

- retval return value of _gtk_text_view_scroll_to_iter wrong in some cases

In addition, this patch re-factor priv->top_border
in screen_dest.y calculation

Of course, all GtkTextView and GtkSourceView based app were impacted
by these bugs ( gedit for example, see bug 754147 )

https://bugzilla.gnome.org/show_bug.cgi?id=753815
Comment 17 Matthias Clasen 2015-09-02 02:48:49 UTC
Attachment 310439 [details] pushed as 9ad6ac0 - GtkTextView: various scroll fixes