This was first mentioned here http://bugzilla.gnome.org/show_bug.cgi?id=579760 and provides similar functionality but having a consistent UI with a very popular program like firefox couldnt hurt.  

I second that. Especially closing windows by middle-clicking on them is a important feature. The whole interface feels very tabby so it should behave accordingly.

Ill work on it at the start of September if no one else does it before then bit busy at the moment.

This is a good idea that brings ease to the process of window-manangement

Created attachment 147114 [details] [review]
Patch for this bug

<Ctrl>+<first mouse button> already in use.
This patch, Bind second mouse button to open an application on a new workspace.

@maxim the patch looks ok at first glance thanks for working on it (I forgot to finish mine because of college stuff) have you tested it?

bug 597320 suggests using middle-click for "open a new window" instead

middle-click opening a new window makes more sense to me.
If you want to open on new workspace you can drag icon to "add workspace" button in bottom right corner.

it's been agreed on bug 597320 that "open a new window" makes more sense

(In reply to comment #9)
> it's been agreed on bug 597320 that "open a new window" makes more sense

Not sure of the history, but it turns out that control-click already means "open a new window", so there's no point in using middle click for that. Reopening this bug.

Review of attachment 147114 [details] [review]:

Patch doesn't seem to correspond to the current code, and also needs to handle BaseAppSearchProvider for a consistent experience.

In that case it makes more sense to use the middlemouse to open a new window than a new window on a new workspace since opening a new window is a far more common action than opening a window on a new workspace. So:
Middlemouse: New window
Ctrl-mouse1 or maybe <other modifier>+mouse1: New window on new workspace

"a new window is a far more
common action than opening a window on a new workspace"

I would argue that.

By using middle for opening on a new workspace we are consistent with modern web browsers and other applications that use tabs and benefit from the behaviour that users have already learnt by using those apps.

Created attachment 158758 [details] [review]
proposed patch

> Patch doesn't seem to correspond to the current code, and also needs to handle
> BaseAppSearchProvider for a consistent experience.

I tried to update the previous patch for this bug. I didn't understand though what needs to be done about BaseAppSearchProvider.

Review of attachment 158758 [details] [review]:

Thank you for the patch! It looks good overall and works as expected. There is just one comment about the commit message and one comment about the code below.

I think what Owen meant by handling BaseAppSearchProvider is that we should make sure that the applications shown in the search results can be opened on the new workspace the same way. In fact, it works with the current patch because AppSearchResultDisplay::renderResults() uses AppWellIcon to render results.

We can later add this functionality to other items in the overview (the code is in GerericDisplayItem in genericDisplay.js), but it might not be worth doing now with the impending re-write of the UI to support the new docs display vision: http://live.gnome.org/GnomeShell/Design/Whiteboards/FindingAndReminding 

The subject and the body that you have for your commit are pretty good, but I would suggest something more active-sounding and detailed, like:

--------------------------------------------

Add a new way to open an application on a new workspace

Allow using the middle mouse button to open a new instance of an application on a new workspace. The middle mouse button function can be achieved by clicking the left and right mouse buttons together with a two buttons mouse and holding Ctrl while clicking with a single button mouse.

--------------------------------------------

Here is an e-mail that describes the commit message style we use:
http://lists.cairographics.org/archives/cairo/2008-September/015092.html

You can also see the latest commits with 'git log' or at http://git.gnome.org/browse/gnome-shell to check out how well they adhere to that commit style among other things :).

::: js/ui/appDisplay.js
@@ +459,3 @@
             this._onActivate(event);
+        } else if (button == 2) {
+            global.screen.append_new_workspace(false, global.get_current_time()).activate(global.get_current_time());

I think it might be cleaner to rename this._workspaces in Overview.js to this.workspaces to indicate that it is a public variable and then use the following here:

Main.overview.workspaces.addWorkspace();

(In reply to comment #15)

> ::: js/ui/appDisplay.js
> @@ +459,3 @@
>              this._onActivate(event);
> +        } else if (button == 2) {
> +            global.screen.append_new_workspace(false,
> global.get_current_time()).activate(global.get_current_time());
> 
> I think it might be cleaner to rename this._workspaces in Overview.js to
> this.workspaces to indicate that it is a public variable and then use the
> following here:
> 
> Main.overview.workspaces.addWorkspace();

It does make more sense to me to have a "workspace manager" handle
adding the new workspace, however addWorkspace() in
workspacesView.js is empty (maybe I'm using an outdated source
tree?). 
Did you mean moving the line:

global.screen.append_new_workspace(false,
global.get_current_time()).activate(global.get_current_time());

in addWorkspace()?

If you search for addWorkspaces() in workspacesView.js, you will see that there are multiple implementations of that function. One in GenericWorkspacesView, one in MosaicView, and one in SingleView. You probably looked at the first one, which is not implemented because GenericWorkspacesView is a base class for the other two.

So Main.overview.workspaces will be set correctly to the current view because it gets it from the WorkspacesManager. You can look at WorkspacesManager::_updateView() for some relevant code. So I'm pretty sure what I suggested originally will work, which is replace the line in (button == 2) case with Main.overview.workspaces.addWorkspace(); and renaming this._workspaces in Overview.js to this.workspaces

The classes in workspacesView.js are actually a good example of how inheritance is done in JavaScript. You can find another example in genericDisplay.js/docDisplay.js/appDisplay.js

Created attachment 160330 [details] [review]
updated patch

I updated the patch according to Marina's comments.

Review of attachment 160330 [details] [review]:

This patch is an interdiff to the previous version - you need to squash both patches (= combine them into a single patch) using something like "git rebase --interactive origin/master".
Apart from that it looks good and works as advertised.

Created attachment 160386 [details] [review]
updated patch

Thanks, I was wondering about that :)
I hope it looks better now.

Review of attachment 160386 [details] [review]:

(In reply to comment #20)
> I hope it looks better now.

Yup, it does...

The formatting of the commit message could be improved - the lines are slightly too long and the bugzilla line is usually preceded by an empty line - apart from that it should be ready to go.

OK, I went ahead and pushed the patch - thanks again for working on this!

Created attachment 160924 [details] [review]
Make the workspace added on the middle mouse button click active

This ensures that we launch the new instance of an appliation on the newly
added workspace.

Created attachment 160925 [details] [review]
Make the workspace added on the middle mouse button click active

This ensures that we launch the new instance of an application on the newly
added workspace in the grid view, in which we don't make the newly added
workspace active by default.

Created attachment 160937 [details] [review]
Don't add a new workspace when the maximum workspaces limit is reached

It was previously possible to add a workspace above the maximum workspaces
limit by dragging an item to the "add workspace" button or using the middle
mouse button click.

Provide the user with feedback in the info bar when it is not possible to create
a new workspace or remove the current workspace.

Review of attachment 160925 [details] [review]:

LGTM

Review of attachment 160937 [details] [review]:

Looks good and works well.

::: js/ui/workspacesView.js
@@ +260,3 @@
+            return;
+            Main.overview.infoBar.setMessage(_("Can't remove the first workspace."));
+        if (!this.canRemoveWorkspace()) {

Trailing whitespace.