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 606460 - need to decide the max number of workspaces allowed
need to decide the max number of workspaces allowed
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal critical
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-01-09 00:43 UTC by Iain Nicol
Modified: 2010-01-20 19:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[overview] disable the AddWorkspaceButton if there are >= 16 workspaces (1.63 KB, patch)
2010-01-18 19:59 UTC, Dan Winship
committed Details | Review
[dnd] Don't allow dropping on non-reactive actors (996 bytes, patch)
2010-01-18 19:59 UTC, Dan Winship
committed Details | Review
[dnd] revert the change to only allow dropping on reactive actors (1.59 KB, patch)
2010-01-20 19:03 UTC, Dan Winship
committed Details | Review

Description Iain Nicol 2010-01-09 00:43:56 UTC
GNOME Shell seems to support adding an arbitrary number of workspaces.  This is bad, because Mutter doesn't; Mutter has the fixed-size array workspace_names[MAX_REASONABLE_WORKSPACES], where MAX_REASONABLE_WORKSPACES is 36.

This can cause a crash.  In GNOME Shell, add loads (at least 36) of workspaces.  Click on any of the workspaces.  Now attempt to switch workspace using the keyboard combo Ctrl-Alt-{Left,Right}; Mutter crashes.  This can't happen in GNOME 2.x, because you can't set there to be more than 36 workspace.

Here are the debug messages:

Window manager warning: Log level 8: meta_prefs_get_workspace_name: assertion `i >= 0 && i < MAX_REASONABLE_WORKSPACES' failed
**
mutter:ERROR:core/screen.c:1669:meta_screen_workspace_popup_create: assertion failed: (entries[i].title != NULL)


What happens is meta_screen_popup_create calls meta_prefs_get_workspace_name (via meta_workspace_get_name) to get the name of a high-index workspace.  _get_workspace_name returns NULL and trips the warning because of the call
    g_return_val_if_fail (i >= 0 && i < MAX_REASONABLE_WORKSPACES, NULL);.
Because Mutter doesn't know the name of the workspace, _popup_create fails the above assertion.

Obviously one solution would be to limit the number of workspaces you can create in gnome-shell.



It's probably not useful, but here's a backtrace just before the crash:

Breakpoint 1, meta_screen_workspace_popup_create (screen=<value optimized out>, initial_selection=<value optimized out>) at core/screen.c:1669
(gdb) bt
  • #0 meta_screen_workspace_popup_create
    at core/screen.c line 1669
  • #1 invoke_handler
    at core/keybindings.c line 1226
  • #2 process_event
    at core/keybindings.c line 1311
  • #3 meta_display_process_key_event
    at core/keybindings.c line 1474
  • #4 event_callback
    at core/display.c line 1722
  • #5 filter_func
    at ui/ui.c line 84
  • #6 gdk_event_apply_filters
    at gdkevents-x11.c line 351
  • #7 gdk_event_translate
    at gdkevents-x11.c line 922
  • #8 _gdk_events_queue
    at gdkevents-x11.c line 2305
  • #9 gdk_event_dispatch
    at gdkevents-x11.c line 2366
  • #10 g_main_dispatch
    at gmain.c line 1960
  • #11 IA__g_main_context_dispatch
    at gmain.c line 2513
  • #12 g_main_context_iterate
    at gmain.c line 2591
  • #13 IA__g_main_loop_run
    at gmain.c line 2799
  • #14 main
    at core/main.c line 686

Comment 1 Dan Winship 2010-01-13 20:44:30 UTC
ok, if metacity actually enforces some maximum number of workspaces, then we need to decide the maximum gnome-shell will support
Comment 2 Iain Nicol 2010-01-17 08:10:41 UTC
Well, if it's *desirable* to support an arbitrary number of workspaces, I think I could come up the patch for Mutter.  AFAICT all that needs done is for workspace_names to become a GPtrArray.  Metacity's schema.in says the number was capped for `performance reasons', which is understandable.

Should I go ahead?  Or would you rather cap?
Comment 3 Milan Bouchet-Valat 2010-01-17 14:29:14 UTC
Who needs more than 36 workspaces, anyway? Just pick MAX_REASONABLE_WORKSPACES for the shell too.
Comment 4 Owen Taylor 2010-01-18 16:26:14 UTC
I think I agree with Dan here that if we are going to put a number in and not just say "if you keep on clicking the + button, you get what you deserve", we should put a number in that makes sense from a user interface perspective.

I'd suggest 16 - the UI really isn't designed for more than that. (If somebody makes a patch, the comment above the constant should mention the hard limit of Metacity's MAX_REASONABLE_WORKSPACES.)

I don't see any reason for a Mutter patch - the current limit is more than anybody could really want.
Comment 5 Dan Winship 2010-01-18 19:59:34 UTC
Created attachment 151702 [details] [review]
[overview] disable the AddWorkspaceButton if there are >= 16 workspaces
Comment 6 Dan Winship 2010-01-18 19:59:38 UTC
Created attachment 151703 [details] [review]
[dnd] Don't allow dropping on non-reactive actors
Comment 7 Owen Taylor 2010-01-18 20:01:37 UTC
Review of attachment 151702 [details] [review]:

Looks good
Comment 8 Owen Taylor 2010-01-18 20:01:59 UTC
Review of attachment 151703 [details] [review]:

Looks good
Comment 9 Dan Winship 2010-01-18 20:14:14 UTC
Attachment 151702 [details] pushed as a50d64e - [overview] disable the AddWorkspaceButton if there are >= 16 workspaces
Comment 10 Dan Winship 2010-01-20 19:03:52 UTC
Created attachment 151867 [details] [review]
[dnd] revert the change to only allow dropping on reactive actors

Workspace.actor is non-reactive, and there's probably some good reason
for this. So just have the add workspace button check its own reactivity.
Comment 11 Marina Zhurakhinskaya 2010-01-20 19:10:37 UTC
Review of attachment 151867 [details] [review]:

Looks good!