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

[Iain Lane]

With gtk 3.16, gnome-terminal's terminal widgets do not have overlay scrollbars.

[Rishi-is (rishi-is)]

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

commit 3dcd0a24b1871c71e667df180334b4b861fbbc52
Author: Matthias Clasen <email address hidden>
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.

[Rishi-is (rishi-is)]

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

[Chpe (chpe)]

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.

[Rishi-is (rishi-is)]

(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?

[Rishi-is (rishi-is)]

Created attachment 297999
screen-container: Remove undefined public method

[Rishi-is (rishi-is)]

Created attachment 298000
screen-container: Remove wrong compiler attribute

[Rishi-is (rishi-is)]

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

[Matthias Clasen (mclasen)]

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...

[Chpe (chpe)]

(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?

[Matthias Clasen (mclasen)]

(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.

[Carlos Garnacho (carlosg)]

(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.

[Chpe (chpe)]

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.

[Matthias Clasen (mclasen)]

(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.

[Chpe (chpe)]

(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.

[Rishi-is (rishi-is)]

(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 ?

[Chpe (chpe)]

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

[Chpe (chpe)]

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

Sure.

[Lars Karlitski (larsu)]

They do for me?! However, they're hardly usable because they don't expand on hover. (I think that's because vte doesn't use GtkScrolledWindow.)

[Iain Lane (laney)]

On Mon, Aug 31, 2015 at 08:43:09AM -0000, Lars Uebernickel wrote:
> They do for me?! However, they're hardly usable because they don't
> expand on hover. (I think that's because vte doesn't use
> GtkScrolledWindow.)

Isn't it that they have the styling from the theme but not the
behaviour? They don't seem to be /overlay/ for me; the content reflows
if I expand them. (I guess because it handles those itself, as you say.)

With our latest theme the behaviour seems better than I recall from
before though.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Egmont Koblinger (egmont-gmail)]

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 ...

[Chpe (chpe)]

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.

[Rishi-is (rishi-is)]

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

[Chpe (chpe)]

(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.

[Chpe (chpe)]

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.

[Matthias Clasen (mclasen)]

(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

[Rishi-is (rishi-is)]

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.

[Rishi-is (rishi-is)]

(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.)

[Rishi-is (rishi-is)]

(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.

[Rishi-is (rishi-is)]

Created attachment 327779
Support animated and touch scrolling

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

[Rishi-is (rishi-is)]

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

[Chpe (chpe)]

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.

[Rishi-is (rishi-is)]

(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.

[Rishi-is (rishi-is)]

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.

[Rishi-is (rishi-is)]

(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.

[Carlos Garnacho (carlosg)]

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.

[Rishi-is (rishi-is)]

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

[Chpe (chpe)]

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.

[Chpe (chpe)]

+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?

[Rishi-is (rishi-is)]

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

[Rishi-is (rishi-is)]

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

[Rishi-is (rishi-is)]

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

[Rishi-is (rishi-is)]

(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

[Chpe (chpe)]

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?

[Rishi-is (rishi-is)]

(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.

[Chpe (chpe)]

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.

[Rishi-is (rishi-is)]

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

Ok, thanks. Pushed to master.

[Chpe (chpe)]

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

[Gnome-sysadmin (gnome-sysadmin)]

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/7479.