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 770430 - [wayland]: GtkPopover size exceeds supported texture size
[wayland]: GtkPopover size exceeds supported texture size
Status: RESOLVED FIXED
Product: gedit
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Gedit maintainers
Gedit maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-26 08:32 UTC by Olivier Fourdan
Modified: 2016-08-29 09:48 UTC
See Also:
GNOME target: 3.22
GNOME version: ---


Attachments
[PATCH] gedit-open-document-selector: Add max-content-height (1.15 KB, patch)
2016-08-29 08:13 UTC, Olivier Fourdan
none Details | Review
[PATCH] gedit-open-document-selector: Add max-content-height (1.26 KB, patch)
2016-08-29 08:57 UTC, Olivier Fourdan
committed Details | Review
[PATCH for 3.20] gedit-open-document-selector: Add max-content-height (1.32 KB, patch)
2016-08-29 09:24 UTC, Olivier Fourdan
none Details | Review

Description Olivier Fourdan 2016-08-26 08:32:39 UTC
Description

This is a follow up on bug 770387 where gedit search in file open popover would crash mutter.

Investigation showed that the buffer height would exceed the supported texture size in some cases, which should not happen.

In the case of bug 770387, the height of the corresponding GdkWindow would exceed 20,000 pixels.

Steps to reproduce:

Same as bug 770387

1. Open gedit
2. Click on "Open"
3. Start typing in the search field in the GtkPopover

Actual result:

The buffer height becomes very large very quickly.

Additional data:

Gtk+ logs a lot of the following messages when this happens:

(gedit:2833): Gtk-WARNING **: GtkSearchEntry 0x99a2b0 is drawn without a current allocation. This should not happen.

(gedit:2833): Gtk-WARNING **: GeditWindow 0x710dc0 is drawn without a current allocation. This should not happen.

(gedit:2833): Gtk-WARNING **: GtkPopover 0x95e460 is drawn without a current allocation. This should not happen.

(gedit:2833): Gtk-WARNING **: GeditOpenDocumentSelector 0x9603a0 is drawn without a current allocation. This should not happen.
Comment 1 Matthias Clasen 2016-08-26 14:22:23 UTC
Playing with this briefly, setting a suitable max-content-height on the scrolled window brings the old behaviour back. Nothing to do with Wayland, per se.
Comment 2 Olivier Fourdan 2016-08-26 15:11:43 UTC
I'm doing a bisect atm, basically 3.21.1 is good and 3.21.3 is bad...
Comment 3 Olivier Fourdan 2016-08-26 16:28:55 UTC
git bisect gives:

d7e242eec03a1056cdc0808aef6bcf1002b4df8b is the first bad commit
commit d7e242eec03a1056cdc0808aef6bcf1002b4df8b
Author: Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
Date:   Tue Jun 7 14:04:42 2016 +0900

    scrolledwindow: Bug 766569 - General size request fixes.
    
    This patch does a couple of things:
    
      o Removes the obscure 'extra_width' and 'extra_height' variables
        making the request code exceedingly difficult to read
    
      o Fixes the max-content-size properties introduced in bug 742281
        so that they do not grow the minimum request.
    
      o Cleanup of request code in general:
        - min/max content sizes are clamped around the child request as needed
        - scrollbar requests are only added in one place, after child request
          sizes are calculated and without the extra_width/height thing.

:040000 040000 5d2bf5a6d8b628a6e7506414390decba610a40b7 322b2128103bad9f97a58b5ea2affcea65ad0126 M	gtk
Comment 4 Matthias Clasen 2016-08-26 17:21:08 UTC
as I said, setting max-content-height fixes things.
This is an (intentional) behavior change in GtkScrolledWindow, I'm afraid
Comment 5 Olivier Fourdan 2016-08-29 08:13:07 UTC
Created attachment 334331 [details] [review]
[PATCH] gedit-open-document-selector: Add max-content-height

(In reply to Matthias Clasen from comment #4)
> as I said, setting max-content-height fixes things.
> This is an (intentional) behavior change in GtkScrolledWindow, I'm afraid

So, move to gedit instead.

For the record: https://blog.gtk.org/2016/06/08/controlling-content-sizes-in-gtkscrolledwindow/

It feels odd to put an arbitrary, hardcoded numeric value here, what would be the "right" value (could depend on DPI, font size, etc.).
Comment 6 Olivier Fourdan 2016-08-29 08:14:40 UTC
BTW, possible patch attached, this indeed fixes the issue with Wayland.
Comment 7 Ignacio Casal Quinteiro (nacho) 2016-08-29 08:19:23 UTC
Review of attachment 334331 [details] [review]:

yeah... this is not very nice. I wonder if we could set it to something like 5 * row height...
Comment 8 Olivier Fourdan 2016-08-29 08:57:14 UTC
Created attachment 334332 [details] [review]
[PATCH] gedit-open-document-selector: Add max-content-height

Alternatively, if we do it programatically, it works equally well and gives the same result as before.
Comment 9 Ignacio Casal Quinteiro (nacho) 2016-08-29 09:04:50 UTC
Review of attachment 334332 [details] [review]:

Looks good to me. Feel free to backport it if needed.
Comment 10 Olivier Fourdan 2016-08-29 09:10:49 UTC
Comment on attachment 334332 [details] [review]
[PATCH] gedit-open-document-selector: Add max-content-height

attachment 334332 [details] [review] pushed to git master as commit d4918b7 - gedit-open-document-selector: Add max-content-height
Comment 11 Olivier Fourdan 2016-08-29 09:24:49 UTC
Created attachment 334339 [details] [review]
[PATCH for 3.20] gedit-open-document-selector: Add max-content-height

(In reply to Ignacio Casal Quinteiro (nacho) from comment #9)
> [...] Feel free to backport it if needed.

The problem with backporting it is that it's a new API added in gtk+-3.21.x so not available in 3.20

The problem occurs when building/shipping gedit-3.20 with gtk+-3.21

Would using GTK_CHECK_VERSION() be acceptable to catch such a case?
Comment 12 Ignacio Casal Quinteiro (nacho) 2016-08-29 09:37:28 UTC
well, if this is something added to gtk+ 3.21 then I would say to not backport it.
Thanks for the patch btw
Comment 13 Olivier Fourdan 2016-08-29 09:48:57 UTC
(In reply to Ignacio Casal Quinteiro (nacho) from comment #12)
> well, if this is something added to gtk+ 3.21 then I would say to not
> backport it.
> Thanks for the patch btw

Yeap agreed, closing then (fix is now in master)