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 672543 - keyring dialog too big
keyring dialog too big
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 644667 672149 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-03-21 12:13 UTC by Matthias Clasen
Modified: 2013-07-02 21:50 UTC
See Also:
GNOME target: 3.4
GNOME version: ---


Attachments
screenshot (178.08 KB, image/png)
2012-03-21 12:16 UTC, Matthias Clasen
  Details
st-box-layout: Don't bypass Clutter's size negotiation (1.28 KB, patch)
2012-03-23 23:56 UTC, Florian Müllner
none Details | Review
checkBox: Work around a height-for-width problem (1.84 KB, patch)
2012-03-26 09:23 UTC, Florian Müllner
committed Details | Review

Description Matthias Clasen 2012-03-21 12:13:55 UTC
Here is what I see with .92
Comment 1 Matthias Clasen 2012-03-21 12:16:39 UTC
Created attachment 210227 [details]
screenshot
Comment 2 Florian Müllner 2012-03-21 13:19:28 UTC
Also see https://bugzilla.gnome.org/show_bug.cgi?id=652459#c13.

I'm pretty sure that this is a problem with StTable not handling size requests of height-for-width actors correctly - in short, the checkbox' height is based on the natural/minimal width of the entry ("the widest actor in the column"), not the actual width we end up allocating.

For 3.4, I'll take a look at the corresponding code in MxTable/ClutterTableLayout, but longer term, my preferred approach would be to subclass ClutterTableLayout (to account for children's CSS properties) and use that in StTable (so we can leave the actual layout logic to the existing ClutterLayoutManager and just keep the widget for convenience/compatibility).
Comment 3 Florian Müllner 2012-03-21 13:20:28 UTC
*** Bug 672149 has been marked as a duplicate of this bug. ***
Comment 4 Cosimo Cecchi 2012-03-21 20:26:38 UTC
I can reproduce this; note that as soon as you start typing some characters in the dialog, its height will shrink down (if you then delete characters, the height will turn back to the initial size again).
Comment 5 Florian Müllner 2012-03-23 23:56:32 UTC
Created attachment 210496 [details] [review]
st-box-layout: Don't bypass Clutter's size negotiation

For horizontal box layouts, we assume children will be given their
preferred width when querying them for their height. While this
assumption makes some sense when considering a single box with its
children, it interferes with Clutter's size negotiation when
dealing with nested BoxLayouts of different orientations.


Apparently I was wrong in blaming StTable. This patch fixes most of the problem for me - there is still some shrinking when underallocating the entry (e.g. entering more characters than which fit the width), but it is a definitive improvement over what we have now.

Still, I'm not quite happy with the patch - it feels dirty to get rid of the enforced -1 for_size in get_content_preferred_height(), but not in get_content_preferred_width(). Unfortunately the latter breaks quite spectacularly, so I'm leaving that out for now.

I think it should be possible to come up with a keyringPrompt-specific work-around in case we consider this patch too risky at this point - I'm sure the change is correct, but there might be places where we rely on the current behavior which I missed in testing.

Opinions?
Comment 6 Florian Müllner 2012-03-26 09:23:40 UTC
Created attachment 210606 [details] [review]
checkBox: Work around a height-for-width problem

StBoxLayout currently does not handle height-for-width children
correctly under some circumstances. As a work-around, hard-code
a label height of two lines of text, which should work for most
locales in the one place the widget is currently used.


Quick and dirty work-around, in case we discard the fix as too risky.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-03-26 09:46:11 UTC
Review of attachment 210606 [details] [review]:

Yeah, let's go with this.
Comment 8 Florian Müllner 2012-03-26 12:45:19 UTC
Comment on attachment 210606 [details] [review]
checkBox: Work around a height-for-width problem

Attachment 210606 [details] pushed as bf99298 - checkBox: Work around a height-for-width problem

Pushed with release team approval.
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-03-26 15:03:33 UTC
Let's mark this as fixed for now. We can review the rest of the stuff for the 3.6 cycle (including porting to ClutterBoxLayout and all that fun stuff)
Comment 10 Florian Müllner 2012-03-26 16:01:09 UTC
(In reply to comment #9)
> Let's mark this as fixed for now. We can review the rest of the stuff for the
> 3.6 cycle (including porting to ClutterBoxLayout and all that fun stuff)

I think the first patch is still worth considering for 3.4.1, see http://mail.gnome.org/archives/release-team/2012-March/msg00301.html (obviously moving to ClutterBoxLayout or anything is out of the question pre-3.5)
Comment 11 Jasper St. Pierre (not reading bugmail) 2013-07-02 21:50:36 UTC
*** Bug 644667 has been marked as a duplicate of this bug. ***