Launchpad has imported 47 comments from the remote bug at
https://bugzilla.gnome.org/show_bug.cgi?id=733210.

If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.

------------------------------------------------------------------------
On 2014-07-15T15:21:24+00:00 Rishi-is wrote:

In gtk+ 3.13.x GtkScrolledWindow animates the scrolling motion. See:

commit 3dcd0a24b1871c71e667df180334b4b861fbbc52
Author: Matthias Clasen <mcla...@redhat.com>
Date:   Mon Jun 30 18:12:39 2014 -0400

    GtkScrolledWindow: Enable animated scrolling
    
    We use gtk_adjustment_enable_animation to enable animated
    updates of the adjustments. Currently, this is enabled
    unconditionally, and with a duration that is hardcoded.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=732376

Touch or thumb scrolling also comes for free with GtkScrolledWindow.

Let's put the VteTerminal widget in a GtkScrolledWindow instead of
creating our own vertical GtkScrollbar, so that we don't have to write
our own code for these.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/0

------------------------------------------------------------------------
On 2014-07-15T15:44:15+00:00 Rishi-is wrote:

Created attachment 280733
screen-container: Support animated and touch scrolling

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/1

------------------------------------------------------------------------
On 2014-08-16T17:12:36+00:00 Chpe wrote:

Comment on attachment 280733
screen-container: Support animated and touch scrolling

I don't like this approach. GtkScrolledWindow is for use when your
content can adapt to size changes, which doesn't work with vteterminal.
Abusing it by hiding its scrollbar behind its back is too hacky.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/2

------------------------------------------------------------------------
On 2015-02-26T13:26:12+00:00 Rishi-is wrote:

(In reply to Christian Persch from comment #2)
> I don't like this approach. GtkScrolledWindow is for use when your content
> can adapt to size changes, which doesn't work with vteterminal.

These days VteTerminal can rewrap the content. Or were you talking about
something else?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/3

------------------------------------------------------------------------
On 2015-02-26T16:00:37+00:00 Rishi-is wrote:

Created attachment 297999
screen-container: Remove undefined public method

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/4

------------------------------------------------------------------------
On 2015-02-26T16:01:07+00:00 Rishi-is wrote:

Created attachment 298000
screen-container: Remove wrong compiler attribute

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/5

------------------------------------------------------------------------
On 2015-02-26T16:01:35+00:00 Rishi-is wrote:

Created attachment 298001
screen-container: Support animated and touch scrolling

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/6

------------------------------------------------------------------------
On 2015-03-02T19:05:29+00:00 Matthias Clasen wrote:

Review of attachment 298001:

Looks good to me, otherwise. Great that the EXTERNAL policy found an
actual use case!

::: src/terminal-screen-container.c
@@ +107,3 @@
+                                  GTK_POLICY_NEVER,
+                                  GTK_POLICY_ALWAYS);
+  gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

it would be more idiomatic to say

gtk_scrolled_window_new (NULL, NULL);
gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

and have the two deal with their adjustments internally...

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/7

------------------------------------------------------------------------
On 2015-03-02T19:06:21+00:00 Matthias Clasen wrote:

Review of attachment 298001:

Looks good to me, otherwise. Great that the EXTERNAL policy found an
actual use case!

::: src/terminal-screen-container.c
@@ +107,3 @@
+                                  GTK_POLICY_NEVER,
+                                  GTK_POLICY_ALWAYS);
+  gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

it would be more idiomatic to say

gtk_scrolled_window_new (NULL, NULL);
gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

and have the two deal with their adjustments internally...

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/8

------------------------------------------------------------------------
On 2015-03-02T19:32:47+00:00 Matthias Clasen wrote:

Review of attachment 298001:

Looks good to me, otherwise. Great that the EXTERNAL policy found an
actual use case!

::: src/terminal-screen-container.c
@@ +107,3 @@
+                                  GTK_POLICY_NEVER,
+                                  GTK_POLICY_ALWAYS);
+  gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

it would be more idiomatic to say

gtk_scrolled_window_new (NULL, NULL);
gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));

and have the two deal with their adjustments internally...

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/9

------------------------------------------------------------------------
On 2015-03-02T21:03:29+00:00 Chpe wrote:

(In reply to Debarshi Ray from comment #3)
> (In reply to Christian Persch from comment #2)
> > I don't like this approach. GtkScrolledWindow is for use when your content
> > can adapt to size changes, which doesn't work with vteterminal.
> 
> These days VteTerminal can rewrap the content. Or were you talking about
> something else?

Let me rephrase this. GtkScrolledWindow's implementation of
GtkWidgetClass::get_preferred_* and ::size_allocate are monstrously
complex, and I don't see any evidence that they handle
GTK_POLICY_EXTERNAL correctly (should be same as NEVER, I think?) nor do
they handle a GTK_SIZE_REQUEST_CONSTANT_SIZE child correctly
(size_allocate uses |if (gtk_widget_get_request_mode (child) ==
GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH) { do HFW } else /*
GTK_SIZE_REQUEST_WIDTH_FOR_HEIGHT */ { do WFH }| so it doesn't appear to
even consider a CONSTANT_SIZE child).

What I would like is a *simple* scroll container that just forwards the
request/allocate to the child and just tacks on the scrollbar(s).

(In reply to Debarshi Ray from comment #0)
>     We use gtk_adjustment_enable_animation to enable animated
>     updates of the adjustments. Currently, this is enabled
>     unconditionally, and with a duration that is hardcoded.

Do I understand correctly what this does: if adjustment is at value v_0
and, either programmatically, or by user interaction, should now be v_1,
the value is animated progressively from v_0 to v_1 instead of being set
directly to v_1?

If so, I don't see how this is a desirable thing for a terminal. This means 
that we'll have to load, decrypt, decompress, decode a whole range of 
intermediate scrollback content just to display it for 1/60th of a second...
can this part be disabled programmatically ?

> Touch or thumb scrolling also comes for free with GtkScrolledWindow.

How does this interact with selection, dingus, or mouse tracking mode?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/10

------------------------------------------------------------------------
On 2015-03-02T21:48:32+00:00 Matthias Clasen wrote:

(In reply to Christian Persch from comment #10)
 
> Let me rephrase this. GtkScrolledWindow's implementation of
> GtkWidgetClass::get_preferred_* and ::size_allocate are monstrously complex,
> and I don't see any evidence that they handle GTK_POLICY_EXTERNAL correctly
> (should be same as NEVER, I think?) 

No. The point of EXTERNAL is very much that it is different from NEVER.
Why else would we add another enum value ? NEVER forces the
scrolledwindow to make the entire child visible. EXTERNAL allows the
child to be scrolled.

I agree that GtkScrolledWindows allocation code is complicated. It could
be much simpler if we gave up on 'traditional' scrollbars and always
used overlays.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/11

------------------------------------------------------------------------
On 2015-03-02T22:14:13+00:00 Carlos Garnacho wrote:

(In reply to Christian Persch from comment #10)
> (In reply to Debarshi Ray from comment #3)
> > (In reply to Christian Persch from comment #2)
> > > I don't like this approach. GtkScrolledWindow is for use when your content
> > > can adapt to size changes, which doesn't work with vteterminal.
> > 
> > These days VteTerminal can rewrap the content. Or were you talking about
> > something else?
> 
> Let me rephrase this. GtkScrolledWindow's implementation of
> GtkWidgetClass::get_preferred_* and ::size_allocate are monstrously complex,
> and I don't see any evidence that they handle GTK_POLICY_EXTERNAL correctly
> (should be same as NEVER, I think?) nor do they handle a
> GTK_SIZE_REQUEST_CONSTANT_SIZE child correctly (size_allocate uses |if
> (gtk_widget_get_request_mode (child) == GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH) {
> do HFW } else /* GTK_SIZE_REQUEST_WIDTH_FOR_HEIGHT */ { do WFH }| so it
> doesn't appear to even consider a CONSTANT_SIZE child).
> 
> What I would like is a *simple* scroll container that just forwards the
> request/allocate to the child and just tacks on the scrollbar(s).

What I think we aim on the GTK+ side is having all scroll/device details
(touch, kinetic scroll, smooth scroll, plain old 4..7 buttons...) in a
single point, instead of duplicated/copied across.

> 
> (In reply to Debarshi Ray from comment #0)
> >     We use gtk_adjustment_enable_animation to enable animated
> >     updates of the adjustments. Currently, this is enabled
> >     unconditionally, and with a duration that is hardcoded.
> 
> Do I understand correctly what this does: if adjustment is at value v_0 and,
> either programmatically, or by user interaction, should now be v_1, the
> value is animated progressively from v_0 to v_1 instead of being set
> directly to v_1?
> 
> If so, I don't see how this is a desirable thing for a terminal. This means
> that we'll have to load, decrypt, decompress, decode a whole range of
> intermediate scrollback content just to display it for 1/60th of a second...
> can this part be disabled programmatically ?

This is just true if changing between distant regions of the buffer. I
wouldn't think this is the most usual operation? Wouldn't this be
acceptable for PgUp/Down increments?

> 
> > Touch or thumb scrolling also comes for free with GtkScrolledWindow.
> 
> How does this interact with selection, dingus, or mouse tracking mode?

Just fine, FYI this is enabled by default, there's good chances this is
a must somewhere else.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/12

------------------------------------------------------------------------
On 2015-03-03T18:44:59+00:00 Chpe wrote:

Actually testing the patch reveals several problems:

* Lots of warnings on console: (gnome-terminal-server:16272): Gtk-
WARNING **: Toplevel size doesn't seem to directly depend on the size of
the geometry widget from gtk_window_set_geometry_hints(). The geometry
widget might not be in the window, or it might not be packed into the
window appropriately

* Creating tabs and switching between them shrinks the window

* Even just running 'ls' at the prompt shrinks the window

I guess the above are all due to the warning.

* Checking "Show scrollbar" in prefs doesn't work anymore: there is no
scrollbar visible while not moving the mouse; when moving the mouse over
the terminal but outside the scrollbar area, a thick rounded rectangle
shows up, which only turns into a (transparent, ugh) scrollbar when the
mouse if over it.

* The scrollbar is now transparent on top of the content, instead of
intransparent and beside the content, as it should be. This means that
(depending on font size), it's impossible to start selecting in the last
1-2 char cells of each row. Filed as bug .

I think both of these can be fixed by setting the overlay-scrolling
property to FALSE, which we should therefore do.

(In reply to Carlos Garnacho from comment #12)
> > > Touch or thumb scrolling also comes for free with GtkScrolledWindow.
> > 
> > How does this interact with selection, dingus, or mouse tracking mode?
> 
> Just fine, FYI this is enabled by default, there's good chances this is a
> must somewhere else.

Have you actually tested this on a touch device? Because I don't see how
it *can* work if the scrolled window is intercepting events over the
terminal.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/13

------------------------------------------------------------------------
On 2015-03-03T18:47:48+00:00 Matthias Clasen wrote:

(In reply to Christian Persch from comment #13)

> * The scrollbar is now transparent on top of the content, instead of
> intransparent and beside the content, as it should be. This means that
> (depending on font size), it's impossible to start selecting in the last 1-2
> char cells of each row. Filed as bug .

Thats not a bug. It wouldn't be an overlay scrollbar if it wasn't, you
know, overlayed.

> 
> Have you actually tested this on a touch device? Because I don't see how it
> *can* work if the scrolled window is intercepting events over the terminal.

It is not intercepting events that are not vertical swipes.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/14

------------------------------------------------------------------------
On 2015-03-03T18:51:42+00:00 Chpe wrote:

(In reply to Matthias Clasen from comment #14)
> Thats not a bug. It wouldn't be an overlay scrollbar if it wasn't, you know,
> overlayed.

Right. I found the overlay-scrolling property and just forgot to take out the 
bit about filing a bug.
 
> > Have you actually tested this on a touch device? Because I don't see how it
> > *can* work if the scrolled window is intercepting events over the terminal.
> 
> It is not intercepting events that are not vertical swipes.

Selecting usually is a vertical swipe. And in (full) mouse tracking
mode, *all* events need to be send to the terminal.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/15

------------------------------------------------------------------------
On 2015-03-10T12:46:55+00:00 Rishi-is wrote:

(In reply to Christian Persch from comment #13)
> Actually testing the patch reveals several problems:
> 
> * Lots of warnings on console: (gnome-terminal-server:16272): Gtk-WARNING
> **: Toplevel size doesn't seem to directly depend on the size of the
> geometry widget from gtk_window_set_geometry_hints(). The geometry widget
> might not be in the window, or it might not be packed into the window
> appropriately
> 
> * Creating tabs and switching between them shrinks the window
>
> * Even just running 'ls' at the prompt shrinks the window

Oops, sorry. I did not test it with tabs. I thought we fixed the
shrinking issue in bug 743395 , but apparently not.

> I guess the above are all due to the warning.

Possibly. They don't look as harmless as I assumed them to be. I will
take a look.

> * Checking "Show scrollbar" in prefs doesn't work anymore: there is no
> scrollbar visible while not moving the mouse; when moving the mouse over the
> terminal but outside the scrollbar area, a thick rounded rectangle shows up,
> which only turns into a (transparent, ugh) scrollbar when the mouse if over
> it. 
> 
> * The scrollbar is now transparent on top of the content, instead of
> intransparent and beside the content, as it should be. This means that
> (depending on font size), it's impossible to start selecting in the last 1-2
> char cells of each row. Filed as bug .
> 
> I think both of these can be fixed by setting the overlay-scrolling property
> to FALSE, which we should therefore do.

How about adding an option in preferences to toggle the overlay-
scrolling ?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/16

------------------------------------------------------------------------
On 2015-04-11T19:57:36+00:00 Chpe wrote:

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

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/17

------------------------------------------------------------------------
On 2015-04-11T19:58:31+00:00 Chpe wrote:

(In reply to Debarshi Ray from comment #16)
> How about adding an option in preferences to toggle the overlay-scrolling ?

Sure.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/18

------------------------------------------------------------------------
On 2015-11-02T21:40:14+00:00 Egmont Koblinger wrote:

I've tried this now (just as a user; haven't looked at the code). A
couple of remarks:


(In reply to Christian Persch from comment #10)

> Do I understand correctly what this does: if adjustment is at value v_0 and,
> either programmatically, or by user interaction, should now be v_1, the
> value is animated progressively from v_0 to v_1 instead of being set
> directly to v_1?
> 
> If so, I don't see how this is a desirable thing for a terminal. This means
> that we'll have to load, decrypt, decompress, decode a whole range of
> intermediate scrollback content just to display it for 1/60th of a second...
> can this part be disabled programmatically ?

This is what happens currently if you drag the legacy scrollbar, and it
was never raised as an issue. The animation is quite smooth with a 10k
scrollback and is a cool eye-candy and is consistent with the rest of
the apps. Consider a file manager with tons of files and icons to
display for them based on mime type, or even actual thumbnail for
pictures. I don't think rendering the content is any cheaper there. So I
wouldn't be worried about this.

I'm not sure if the scrollbar's implementation is clever enough (but if
not then it should be made smarter) to skip frames if the app is slow
rendering.


> How does this interact with selection, dingus, or mouse tracking mode?

I haven't seen any problem here (apart from the obvious that starting
the selection in the last column becomes troublesome).


(In reply to Carlos Garnacho from comment #12)

> > If so, I don't see how this is a desirable thing for a terminal. This means
> > that we'll have to load, decrypt, decompress, decode a whole range of
> > intermediate scrollback content just to display it for 1/60th of a second...
> > can this part be disabled programmatically ?
> 
> This is just true if changing between distant regions of the buffer. I
> wouldn't think this is the most usual operation? Wouldn't this be acceptable
> for PgUp/Down increments?

You're right. For PgUp/Down increments we hardly ever
load/decrypt/decompress, since the most recently read 64 kB chunk of the
scrollback buffer is cached in its decrypted/decompressed form.


By the way: PgUp/Down animates the scrolling in Files, but not in g-t 
(Shift-PgUp/Down). No clue what it'd take source code wise to change it, but it 
would be cool to see that behavior.


(In reply to Christian Persch from comment #15)

> Selecting usually is a vertical swipe.

Could you please elaborate? Isn't that a vertical simple mouse movement?
(mouse: move vs. scroll wheel; touchpad: one vs. two fingers)


> And in (full) mouse tracking mode,
> *all* events need to be send to the terminal.

I see no behavioral change here. In mouse tracking mode, vertical
scrolls are sent to the terminal.


(In reply to Debarshi Ray from comment #16)

> How about adding an option in preferences to toggle the overlay-
scrolling ?

Do we need that?  Even though not user-friendly to set, making an
overlay scrollbar non-overlay is just a matter of CSS.  E.g. set the
overlay scrollbar's width to 5px and vte's right padding to 6px, voilĂ ,
it no longer overlays the actual content :)


One thing I find terribly irritating, but it could also be just a CSS issue. At 
least in Ubuntu Wily's default desktop (Unity, Ambiance), the scrollbar appears 
as soon as I move the mouse or produce output (the adjustment's parameters 
change), and disappears after a short inactivity. This results in a continuous 
flicker of a prominent orange-ish color.

My expectation: moving the mouse somewhere not over the scrollbar, or
changing the terminal's contents (and implicitly scrolling by producing
output) shouldn't change the look of the scrollbar, apart from the
necessary adjustment of its height. A prominent change in appearance
(color and/or width) should only occur if I move the mouse there, or
scroll explicitly with a keycombo. Is it solveable with overlay
scrollbar?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/22

------------------------------------------------------------------------
On 2016-02-21T15:32:39+00:00 Chpe wrote:

So does this work better with gtk+ master now that geometry support has
been broken? In any case, there are some things that need to be done:

* GtkScrolledWindow need to be fixed to support
GTK_SIZE_REQUEST_CONSTANT_SIZE widgets.

* We can't use GtkScrolledWindow unconditionally, since it will break
(at least) on older gtk, so commit
41eb2ec73653adc5fd36fa73cebdc8a6169f8516 reverted and made to only use
GtkScrolledWindow on new gtk.

* Need to confirm this does not break selection, nor mouse tracking
mode.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/23

------------------------------------------------------------------------
On 2016-05-12T09:23:20+00:00 Rishi-is wrote:

Since VteTerminal handles GtkWidget::scroll-event, putting it inside a
GtkScrolledWindow is not enough to make kinetic scrolling work. See bug
721893

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/24

------------------------------------------------------------------------
On 2016-05-12T11:55:00+00:00 Chpe wrote:

(In reply to Carlos Garnacho from comment #12)
> > If so, I don't see how this is a desirable thing for a terminal. This means
> > that we'll have to load, decrypt, decompress, decode a whole range of
> > intermediate scrollback content just to display it for 1/60th of a second...
> > can this part be disabled programmatically ?
> 
> This is just true if changing between distant regions of the buffer. I
> wouldn't think this is the most usual operation? Wouldn't this be acceptable
> for PgUp/Down increments?

I'm mostly concerned about this for when you've scrolled way up, and
then we snap to bottom on output or keypress. So IMHO setting the
adjustment programmatically without direct user interaction via the
scrollbar (or kinetic once that's enabled) should *never* animate.

(In reply to Debarshi Ray from comment #21)
> Since VteTerminal handles GtkWidget::scroll-event, putting it inside a
> GtkScrolledWindow is not enough to make kinetic scrolling work. See bug
> 721893

Let's not get sidetracked too much here. I think we should *first* use
GtkScrolledWindow as the container (without overlay scrollbars), and
only as follow-ups worry about making kinetic scrolling work and
enabling overlay scrollbars.

So for simply making use of GtkScrolledWindow, comment 21 and bits of
comment 13 need to be addressed.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/25

------------------------------------------------------------------------
On 2016-05-12T11:57:43+00:00 Chpe wrote:

Oh, and we need a way to remove those annoying dashed lines that are
added at the borders of the GtkScrolledWindow if there's content
scrolled out in that direction.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/26

------------------------------------------------------------------------
On 2016-05-12T12:26:24+00:00 Matthias Clasen wrote:

(In reply to Christian Persch from comment #23)
> Oh, and we need a way to remove those annoying dashed lines that are added
> at the borders of the GtkScrolledWindow if there's content scrolled out in
> that direction.

Use css

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/27

------------------------------------------------------------------------
On 2016-05-12T19:15:26+00:00 Rishi-is wrote:

Created attachment 327735
Support animated and touch scrolling

I have reworked the patch against gtk-3-20.

It needs the first two gnome-terminal patches from bug 760944

Creating tabs doesn't shrink the window anymore. The window expands on
the creation of the first tab, and then shrinks back to the old height
when the last tab is gone. No more WARNINGS.

Overlay scrolling has been switched off.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/28

------------------------------------------------------------------------
On 2016-05-12T19:28:57+00:00 Rishi-is wrote:

(In reply to Christian Persch from comment #20)
> * We can't use GtkScrolledWindow unconditionally, since it will break (at
> least) on older gtk, so commit 41eb2ec73653adc5fd36fa73cebdc8a6169f8516
> reverted and made to only use GtkScrolledWindow on new gtk.

I assume that you are worried about gtk+ < 3.20, right?

> * Need to confirm this does not break selection, nor mouse tracking
mode.

Mouse tracking mode should work because VteTerminal steals the GtkWidget
::scroll-event from the GtkScrolledWindow. (This is why kinetic
scrolling is not working at the moment.)

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/29

------------------------------------------------------------------------
On 2016-05-13T10:20:00+00:00 Rishi-is wrote:

(In reply to Matthias Clasen from comment #9)
> Review of attachment 298001 [details] [review]:
> ::: src/terminal-screen-container.c
> @@ +107,3 @@
> +                                  GTK_POLICY_NEVER,
> +                                  GTK_POLICY_ALWAYS);
> +  gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));
> 
> it would be more idiomatic to say
> 
> gtk_scrolled_window_new (NULL, NULL);
> gtk_container_add (GTK_CONTAINER (priv->sw), GTK_WIDGET (priv->screen));
> 
> and have the two deal with their adjustments internally...

Passing NULL and letting GtkScrolledWindow create a new GtkAdjustment
and pass it to VteTerminal is not working the same way as getting the
GtkAdjustment out of VteTerminal and giving it to GtkScrolledWindow. A
brief investigation suggests that the page-increment is different, which
causes it to scroll by 1 line instead of page-increment/10 lines when
using the mouse's scroll wheel.

One could say that it is a bug in the GtkScrollable implementation. For
the purposes of this bug, let's keep doing what the old has been doing
so far. ie. the latter option.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/30

------------------------------------------------------------------------
On 2016-05-13T10:49:10+00:00 Rishi-is wrote:

Created attachment 327779
Support animated and touch scrolling

* Remove undershoot indicators.
* Ensure GtkScrolledWindow is only used with newer gtk+

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/31

------------------------------------------------------------------------
On 2016-05-13T11:00:38+00:00 Rishi-is wrote:

This also gives us smooth sub-cell scrolling with touch pads. See bug
746690

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/32

------------------------------------------------------------------------
On 2016-05-15T18:41:52+00:00 Chpe wrote:

Review of attachment 327779:

Before I review this more in depth, I have a question:

> GtkScrolledWindow doesn't use the natural height and width
> of its child widget if the scrollbar policy is not NEVER. Instead it
> uses the minimum size. We don't want that because the minimum size of
> a VteTerminal is 1x1.

Wouldn't an adding support to GtkScrolledWindow for
GTK_SIZE_REQUEST_CONSTANT_SIZE widget looks essentially like this new
code in terminal-scrolled-window? If so, I'd really prefer this part to
be done inside gtk+.

> we override GtkScrolledWindow's
> get_preferred_height and get_preferred_width virtual methods to make
> it use the natural size.

This will still run GtkScrolledWindow's size-allocate function which
doesn't handle CONSTANT_SIZE children...

::: src/terminal-screen-container.c
@@ +21,3 @@
 #include "terminal-debug.h"
 
+#if GTK_CHECK_VERSION (3, 19, 5)

Let's do

#if gtk 3.19.5
#define USE_SCROLLED_WINDOW
#endif

and then use that new define everywhere below. Makes it easier by just
having to change one place when one wants to try both code paths.

@@ +134,3 @@
+
+  gtk_container_add (GTK_CONTAINER (container), priv->scrolled_window);
+  gtk_widget_show_all (priv->scrolled_window);

I prefer individual gtk_widget_show() for each widget instead of show-
all.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/33

------------------------------------------------------------------------
On 2016-05-16T12:34:27+00:00 Rishi-is wrote:

(In reply to Christian Persch from comment #30)
> Review of attachment 327779 [details] [review]:
> 
> Before I review this more in depth, I have a question:
> 
> > GtkScrolledWindow doesn't use the natural height and width
> > of its child widget if the scrollbar policy is not NEVER. Instead it
> > uses the minimum size. We don't want that because the minimum size of
> > a VteTerminal is 1x1.
> 
> Wouldn't an adding support to GtkScrolledWindow for
> GTK_SIZE_REQUEST_CONSTANT_SIZE widget looks essentially like this new code
> in terminal-scrolled-window? If so, I'd really prefer this part to be done
> inside gtk+.
> 
> > we override GtkScrolledWindow's
> > get_preferred_height and get_preferred_width virtual methods to make
> > it use the natural size.
> 
> This will still run GtkScrolledWindow's size-allocate function which doesn't
> handle CONSTANT_SIZE children...

I actually wrote a quick patch against gtk_scrolled_window_allocate for
GTK_SIZE_REQUEST_CONSTANT_SIZE, but that didn't do the trick. I will
attach it shortly.

As far as I understand, size-allocate is called after the size has
already been allocated. The way GtkScrolledWindow's
get_preferred_height/_width works, by the time we are in
gtk_scrolled_window_allocate, the allocated size is already too small.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/34

------------------------------------------------------------------------
On 2016-05-16T12:36:00+00:00 Rishi-is wrote:

Created attachment 327973
[gtk+] Handle GTK_SIZE_REQUEST_CONSTANT_SIZE

This is the rough gtk+ patch that I sketched out to see if
GtkScrolledWindow can be improved for GTK_SIZE_REQUEST_CONSTANT_SIZE
children. It didn't make any difference.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/35

------------------------------------------------------------------------
On 2016-05-16T12:42:58+00:00 Rishi-is wrote:

(In reply to Debarshi Ray from comment #29)
> This also gives us smooth sub-cell scrolling with touch pads. See bug 746690

Scratch this. Sub-cell scrolling with touch pads already works since
0.44. Sorry for the confusion.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/36

------------------------------------------------------------------------
On 2016-05-16T15:38:44+00:00 Carlos Garnacho wrote:

AFAICS the size requisition code in attachment 327779 is virtually the
same than gtk_scrolled_window_measure(), except:

https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c#n1751 and
https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c#n1777

It seems that GtkScrolledWindow falls in this case through the last
branch (vertical scrollbar is visible through policy and you disable
overlay indicators), so GtkScrolledWindow settles for the
minimum/natural scrollbar height despite the child size.

I'm tbh unsure if GTK_SIZE_REQUEST_CONSTANT_SIZE is meant to magically
fix this or is rather somewhat tangential, as the choice to obey
scrollbar or content requisitions are not based on size or request
modes, rather policy ones.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/37

------------------------------------------------------------------------
On 2016-05-17T15:08:22+00:00 Rishi-is wrote:

VteTerminal uses GTK_SCROLL_NATURAL by default, unlike all the
GtkScrollable implementations inside gtk+. Therefore, one option is to
special-case GTK_SCROLL_NATURAL children in GtkScrolledWindow's size
requisition code. See bug 766569

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/38

------------------------------------------------------------------------
On 2016-08-01T17:38:02+00:00 Chpe wrote:

Comment on attachment 327779
Support animated and touch scrolling

I've committed a version of this part, minus the terminal-scrolled-
window.[ch] files, disabled (#if 0) by default. To enable this (changing
to a #if GTK_CHECK_VERSION(whatever)), GtkScrolledWindow needs to be
fixed.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/39

------------------------------------------------------------------------
On 2016-09-04T13:00:22+00:00 Chpe wrote:

+terminal-window scrolledwindow undershoot.top,
+terminal-window scrolledwindow undershoot.bottom {
+  background: none;
+}

BTW: is this guaranteed to completely disable this (including the
repeated invalidations from the animation), or might themes do something
beyond background on overshoot/undershoot? Ie wouldn't be display:none
be more appropriate for this?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/40

------------------------------------------------------------------------
On 2016-11-04T12:38:03+00:00 Rishi-is wrote:

Created attachment 339114
screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/41

------------------------------------------------------------------------
On 2016-11-04T12:39:54+00:00 Rishi-is wrote:

Created attachment 339116
screen-container: Bubble the screen's size through the scrolled window

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/42

------------------------------------------------------------------------
On 2016-11-04T12:41:22+00:00 Rishi-is wrote:

Created attachment 339117
screen-container: Enable GtkScrolledWindow when gtk+ >= 3.22.0

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/43

------------------------------------------------------------------------
On 2017-10-20T16:22:10+00:00 Rishi-is wrote:

(In reply to Debarshi Ray from comment #38)
> Created attachment 339114 [details] [review]
> screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5

Hey Christian, we should probably merge this because it fixes up commit
0dd655c569f8b81

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/44

------------------------------------------------------------------------
On 2017-10-20T17:42:50+00:00 Chpe wrote:

Comment on attachment 339114
screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5

Hmm... why move the code around? AFAICT just removing the #ifdef around
the widget_class declaration should fix this, shouldn't it?

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/45

------------------------------------------------------------------------
On 2017-10-20T18:07:03+00:00 Rishi-is wrote:

(In reply to Christian Persch from comment #42)
> Comment on attachment 339114 [details] [review]
> screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5
> 
> Hmm... why move the code around? AFAICT just removing the #ifdef around the
> widget_class declaration should fix this, shouldn't it?

Mainly because !USE_SCROLLED_WINDOW and GTK_CHECK_VERSION(3, 19, 5)
cases are mutually independent. I am happy to re-arrange it differently.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/46

------------------------------------------------------------------------
On 2017-10-20T18:20:13+00:00 Chpe wrote:

Comment on attachment 339114
screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5

Ok, don't bother, just commit as-is.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/47

------------------------------------------------------------------------
On 2017-10-22T07:42:22+00:00 Rishi-is wrote:

Comment on attachment 339114
screen-container: Fix build with USE_SCROLLED_WINDOW and gtk+ >= 3.19.5

Ok, thanks. Pushed to master.

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/48

------------------------------------------------------------------------
On 2019-11-20T22:46:37+00:00 Chpe wrote:

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

Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
terminal/+bug/1451924/comments/49


** Changed in: gnome-terminal
       Status: Unknown => Confirmed

** Changed in: gnome-terminal
   Importance: Unknown => Medium

** Bug watch added: bugzilla.gnome.org/ #732376
   https://bugzilla.gnome.org/show_bug.cgi?id=732376

-- 
You received this bug notification because you are a member of Ubuntu
Desktop Bugs, which is subscribed to gnome-terminal in Ubuntu.
https://bugs.launchpad.net/bugs/1451924

Title:
  gnome-terminal doesn't have overlay scrollbars (that actually overlay
  the content)

To manage notifications about this bug go to:
https://bugs.launchpad.net/gnome-terminal/+bug/1451924/+subscriptions

-- 
desktop-bugs mailing list
desktop-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/desktop-bugs

Reply via email to