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 723496 - Overview is clipped/incomplete. Cannot access lower part of Dash or Grid-Button
Overview is clipped/incomplete. Cannot access lower part of Dash or Grid-Button
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: overview
3.10.x
Other Linux
: Normal major
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 721698 723123 728903 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-02-03 00:01 UTC by Peter
Modified: 2015-03-05 01:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot of clipped overview (1.14 MB, image/png)
2014-02-03 00:01 UTC, Peter
  Details
appDisplay: Don't grow indicators more than the parent (4.13 KB, patch)
2014-06-26 23:02 UTC, Carlos Soriano
none Details | Review
appDisplay: Don't grow indicators more than the parent (3.88 KB, patch)
2014-06-27 08:30 UTC, Carlos Soriano
none Details | Review
appDisplay: Don't grow indicators more than the parent (3.87 KB, patch)
2014-06-27 08:33 UTC, Carlos Soriano
needs-work Details | Review
appDisplay: Don't grow indicators more than the parent (2.59 KB, patch)
2014-08-05 09:20 UTC, Carlos Soriano
committed Details | Review

Description Peter 2014-02-03 00:01:51 UTC
Created attachment 267878 [details]
screenshot of clipped overview

The overview is layed out as if the screen height was substantially larger than it is really.
The lower 1/3 of the overview is invisible and unreachable by the mouse.
Also the Dash (which i suppose is always centered vertically on screen) is clipped so the lower App-Icons and the Grid-Icon is unreachable because the mouse pointer cannot move beyond lower edge.

This looks like gnome-shell had the wrong information about screen height.

Using Fedora20 x86_64 on 3 machines with different screen sizes: 1920x1080, 1680x1050, 2560x1440
gnome-shell 3.10.3-2.fc20
clutter 1.16.2-3.fc20
kernel 3.12.8-300.fc20
xorg-x11-drv-nouveau 1.0.9-2.fc20


Further observations:

All other applications have the correct information about screen size.
The notification area shows on the right place.

The clipping did not occur immediately after OS-installation (Jan.8) but some weeks later on a machine with RadeonHD4650 and now on a second machine with NVidiaQuadroK3000M. All machines using the open source drivers. All machines being on the same update level. The 3rd machine does not (yet) show the error.

Suspecting the error is correlated with one of the later updates i did 'yum downgrade xxx' on all relevant packages but no luck.

Creating a new account and logging into shows the error immediately.

Logging into a seldom used account did _not_ show the error; but editing the dash by adding something to favorites immediately produces the error.

see attached screenshot

The only way to start the missing applications is now by the search function.
Comment 1 MT 2014-05-03 16:33:27 UTC
I've occasionally encountered this bug before, but after upgrading to GNOME 3.12, it's occurring all the time. It starts off fine at the beginning (or after a reset of the shell), but very soon after using the system for a while, I see the exact same behaviour as the original bug reporter. I can reliably trigger it by opening the applications list in the overview, and when I return to the overview, the dash icons and the window thumbnails get pushed off the bottom of the screen (same with the applications list icons too after this).
Comment 2 MT 2014-06-14 11:26:34 UTC
Some further investigation has shown that it happens when elements to the right of the screen are exposed, such as the workspace thumbnail list. As long as I'm using a single workspace, and keep the cursor away from the right of the screen, which keeps the thumbnails hidden, the overview display stays normal.
Comment 3 Rui Matos 2014-06-15 12:01:52 UTC
This might be related to bug 728903 .
Comment 4 Peter 2014-06-16 09:35:04 UTC
I looked at bug 728903 and it shows the same symptoms.
On Andreas' screen-video you see the same "sunken" dashboard with unreachable grid-button and also the preview-windows on the lower egde are partially invisible.
The "wobbling" he is mentioning is a rare symptom. I think the wobbling is a different bug because i see it only sometimes and occasionally also on my last "sane" machine.

The wobbling may be induced be the screen-size-bug as the layouter may consider another arrangement of previews, and while re-arranging, the (wrong) screensize changes and it has to re-arrange again and again.

Another observation:
If the workspace preview is enlarged, the "wrongness" of the screen-size changes and the previews are re-arranged. This is reversible.

On the 2560x1440 screen, the workspace preview is always enlarged and the bug never shows.
On the machines 1920x1080 and 1680x1050 the workspace preview initially comes small and the screen-size-bug shows always.
Comment 5 Carlos Soriano 2014-06-26 11:10:07 UTC
*** Bug 728903 has been marked as a duplicate of this bug. ***
Comment 6 Carlos Soriano 2014-06-26 23:02:00 UTC
Created attachment 279348 [details] [review]
appDisplay: Don't grow indicators more than the parent

Currently the indicators are a box layout inside a BinLayout in AllView.
BinLayout doesn't have any constraint, so if the indicators request a
bigger size than AllView the entire overview is grown, causing overview
to go crazy.

To avoid that, create an actor for the page indicators that request as
minimum size just one indicator, and as a natural size, the sum of all
indicators natural sizes.
In allocation just skip to allocate those indicators that remain outside
the parent.
Comment 7 Carlos Soriano 2014-06-26 23:06:04 UTC
A better approach would require design team input, and I remember the past summer that they were not very convinced to allow let's say more than 192 apps (8 indicators * 24 apps each page).
This is a extreme case, probably with a mixture of low resolution screen, 768 px height, which unfortunately is not that uncommon on semi-new laptops, and a big amount of apps.
Comment 8 Carlos Soriano 2014-06-27 08:30:46 UTC
Created attachment 279363 [details] [review]
appDisplay: Don't grow indicators more than the parent

Currently the indicators are a box layout inside a BinLayout in AllView.
BinLayout doesn't have any constraint, so if the indicators request a
bigger size than AllView the entire overview is grown, causing overview
to go crazy.

To avoid that, create an actor for the page indicators that request as
minimum size just one indicator, and as a natural size, the sum of all
indicators natural sizes.
In allocation just skip to allocate those indicators that remain outside
the parent.
Comment 9 Carlos Soriano 2014-06-27 08:33:09 UTC
Created attachment 279364 [details] [review]
appDisplay: Don't grow indicators more than the parent

Currently the indicators are a BoxLayout inside a BinLayout in AllView.
BinLayout doesn't have any size constraint, so if the indicators request
a bigger size than AllView the entire overview is grown, causing
the overview to go crazy.

To avoid that, create an actor for the page indicators that request as
minimum size 0, and as a natural size, the sum of all indicators natural
sizes.
In allocation just skip to allocate those indicators that remain outside
the parent.
Comment 10 Florian Müllner 2014-07-17 12:28:46 UTC
Review of attachment 279364 [details] [review]:

::: js/ui/appDisplay.js
@@ +196,3 @@
+    },
+
+    vfunc_allocate: function(box, flags) {

You are doing this to avoid partially cut off indicators? Delegating size negotiation to a layout manager and then ignoring it completely for allocation is extremely fishy, not to mention that you ignore spacing and all child flags. I don't think looking a bit less broken in a still pretty broken situation is worth it (if we want to support page overflow properly, we'd have to do something else anyway, like scrolling the indicators or add web-like "ellipsization" ([1] [2] ... [4] >[5]< [6] ... [18]))

@@ +229,3 @@
+        let [, natHeight] = this.parent(forWidth);
+        let themeNode = this.get_theme_node();
+        [, natHeight] = themeNode.adjust_preferred_height(0, natHeight);

The parent method already adjusts the height
Comment 11 Carlos Soriano 2014-07-19 10:27:45 UTC
(In reply to comment #10)
> Review of attachment 279364 [details] [review]:
> 
> ::: js/ui/appDisplay.js
> @@ +196,3 @@
> +    },
> +
> +    vfunc_allocate: function(box, flags) {
> 
> You are doing this to avoid partially cut off indicators? Delegating size
yes
> negotiation to a layout manager and then ignoring it completely for allocation
> is extremely fishy, not to mention that you ignore spacing and all child flags.
right, didn't think how fishy it is actually.
About spacing, you mean just in case we want to add it in a future no?
And about child flags, you mean expand_x, etc just in case for future no?
I though the overall approach was more or less ok since it was DashActor does (more or less)
> I don't think looking a bit less broken in a still pretty broken situation is
> worth it (if we want to support page overflow properly, we'd have to do
> something else anyway, like scrolling the indicators or add web-like
> "ellipsization" ([1] [2] ... [4] >[5]< [6] ... [18]))
> 
But...we agreed to just clip them as a solution no? At least it doesn't make the overview unusable. Other solutions would require design input, and we agreed to go in this way, I am remembering wrong?
> @@ +229,3 @@
> +        let [, natHeight] = this.parent(forWidth);
> +        let themeNode = this.get_theme_node();
> +        [, natHeight] = themeNode.adjust_preferred_height(0, natHeight);
> 
> The parent method already adjusts the height
oh right.
Comment 12 Florian Müllner 2014-07-21 09:32:17 UTC
(In reply to comment #11)
> About spacing, you mean just in case we want to add it in a future no?
> And about child flags, you mean expand_x, etc just in case for future no?

Yes - you inherit from StBoxLayout, so adding spacing is just a question of adding some CSS. I'm not actually asking you to add support for all this, just pointing out that a proper allocate() implementation would replicate much of clutter_box_layout_allocate() (though ignoring the fill/align properties is OK for a custom widget)


> I though the overall approach was more or less ok since it was DashActor does
> (more or less)

DashActor is a bit more specialized (e.g. fixed number of two children) and doesn't expose the spacing property to CSS, but yes, the combination of layout manager and manual allocation is quite questionable there as well.


> But...we agreed to just clip them as a solution no? At least it doesn't make
> the overview unusable.

Yes. I'm just trying to say that clipping won't make the crazy-number-of-apps case work perfectly, regardless of whether we end up with partially clipped indicators or not - as you say, it is an extreme case, so an imperfect solution is fine. I just don't think that partially clipped indicators are that much more imperfect to keep the allocation override (in contrast to the DashActor, where making sure that the all-apps button is unclipped is the whole point of the custom actor - see bug 683340)
Comment 13 Peter 2014-07-21 11:11:45 UTC
Is this bug triggered by (487 > crazy-number-of-apps) ?
What is the exact value of crazy-number-of-apps?

This could explain why the clipping did not occur immediately after OS-installation but later, when some magic number of apps where installed.
Comment 14 Florian Müllner 2014-07-21 12:45:22 UTC
(In reply to comment #13)
> Is this bug triggered by (487 > crazy-number-of-apps) ?
> What is the exact value of crazy-number-of-apps?

There is no exact number - the issue is that the page indicators on the right force the entire overview to grow when there are "too many" pages. How many pages are "too many" depends on the available screen height and the number of apps per page (usually 24 unless for very small screen sizes).
Comment 15 Peter 2014-07-21 14:13:29 UTC
For verifying this idea i did this:
sudo chmod a+w /usr/share/gnome-shell/js/ui/appDisplay.js

Then in the above file around line 32 i set 
const MAX_COLUMNS = 13
const MIN_ROWS = 7

After re-login, the bug is gone! 
All Windows are visible in overview, dashboard is visible on correct position and grid-button is accessible.

As a plus, the grid-display now shows 91 icons instead of 24 which is much more useful then the usual 24.

For verifying even more, i did
const MAX_COLUMNS = 4
const MIN_ROWS = 3
on the 2560x1440 machine which did not show the error, and as expected, the bug emerged.
After re-setting it to 13x7, no more bug.

So for most people waiting for help this patch should do it as workaround.
No need for a quick-and-dirty solution.
Comment 16 nicolas.vieville 2014-07-21 16:39:34 UTC
Hello,

I come from bug report 728903 marked as duplicate of this one. I can confirm that tip given in comment #15 is working here with Fedora 20 x86_64 and gnome-shell 3.10 on 1366x768 display (see https://bugzilla.gnome.org/show_bug.cgi?id=728903#c2 for détails). 
Thanks for sharing this precious experience.

Cordially,


-- 
NVieville
Comment 17 Carlos Soriano 2014-08-05 08:32:26 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > About spacing, you mean just in case we want to add it in a future no?
> > And about child flags, you mean expand_x, etc just in case for future no?
> 
> Yes - you inherit from StBoxLayout, so adding spacing is just a question of
> adding some CSS. I'm not actually asking you to add support for all this, just
> pointing out that a proper allocate() implementation would replicate much of
> clutter_box_layout_allocate() (though ignoring the fill/align properties is OK
> for a custom widget)
> 
> 
> > I though the overall approach was more or less ok since it was DashActor does
> > (more or less)
> 
> DashActor is a bit more specialized (e.g. fixed number of two children) and
> doesn't expose the spacing property to CSS, but yes, the combination of layout
> manager and manual allocation is quite questionable there as well.
> 
> 
> > But...we agreed to just clip them as a solution no? At least it doesn't make
> > the overview unusable.
> 
> Yes. I'm just trying to say that clipping won't make the crazy-number-of-apps
> case work perfectly, regardless of whether we end up with partially clipped
> indicators or not - as you say, it is an extreme case, so an imperfect solution
> is fine. I just don't think that partially clipped indicators are that much
> more imperfect to keep the allocation override (in contrast to the DashActor,
> where making sure that the all-apps button is unclipped is the whole point of
> the custom actor - see bug 683340)

Sorry for the delay, I'm still having problems with receiving emails from bugzilla. I just entered this bug accidentally and saw your comments.

Ok all understood and I agree with you in every point, I'll update the patch.
Comment 18 Carlos Soriano 2014-08-05 09:20:16 UTC
Created attachment 282528 [details] [review]
appDisplay: Don't grow indicators more than the parent

Currently the indicators are a BoxLayout inside a BinLayout in AllView.
BinLayout doesn't have any size constraint, so if the indicators request a
bigger size than AllView the entire overview is grown, causing the overview to
go crazy.

To avoid that, create an actor for the page indicators that request as minimum
size 0, and as a natural size, the sum of all indicators natural sizes. Then we
clip_to_allocation, so it doesn't grow more than the parent.
Comment 19 Florian Müllner 2014-08-05 09:42:17 UTC
Review of attachment 282528 [details] [review]:

Please re-wrap the commit message to not require horizontal scrolling in a 80-character terminal, otherwise looks good.
Comment 20 Carlos Soriano 2014-08-06 07:48:50 UTC
Pushing for now. I'll improve it with design input on a later moment.
At least we don't break overview.

Attachment 282528 [details] pushed as 84cbbaf - appDisplay: Don't grow indicators more than the parent
Comment 21 Florian Müllner 2015-03-05 01:06:46 UTC
*** Bug 723123 has been marked as a duplicate of this bug. ***
Comment 22 Florian Müllner 2015-03-05 01:21:51 UTC
*** Bug 721698 has been marked as a duplicate of this bug. ***