Created attachment 160463 [details] [review]
[altTab] Allow selection of individual windows

Review of attachment 160463 [details] [review]:

::: js/ui/altTab.js
@@ +143,3 @@
         this._appIcons = this._appSwitcher.icons;
 
+

unnecessary blank line

@@ +217,3 @@
         else if (this._thumbnailsFocused) {
             if (keysym == Clutter.Tab) {
+                if (shift && this._currentWindow == 0 || this._currentWindow == -1)

use parens rather than relying on the order of operations between && and ||

@@ -234,3 +237,3 @@
                 this._select(this._nextApp());
             else if (keysym == Clutter.Down || keysym == Clutter.s)
-                this._select(this._currentApp, this._currentWindow);
+                this._select(this._currentApp, 0);

In the case where you're toggling between two windows in the current app, this change means that "Alt-Tab" will show the thumbnail list, with the second window highlighted, but then typing Down will select the first (current) window rather than the second one.

Assuming that wasn't intended, the fix is to obey this._currentWindow if it's not -1.

Created attachment 160591 [details] [review]
[altTab] Allow selection of individual windows

Per design discussion, change things back so that when choosing
an individual window, we raise only that window.  However
when we select an application, raise all windows.

A behavioral change required to clearly differentiate these
is that when the window thumbnail list is popped up, it no
longer has the first window selected by default.  Therefore
the user has to explicitly press the down arrow or use the
mouse to enter individual window selection mode.

Comment on attachment 160591 [details] [review]
[altTab] Allow selection of individual windows

This still has a behavioral change that I'm not sure was intentional.

In the current UI, "Alt-Tab, down" always selects the same window that would have been selected if you'd just done "Alt-Tab". With this patch, "Alt-Tab, down" selects the *next* window (just like "Alt-Tab, backtick" does).

If that was on purpose, then lgtm.

(In reply to comment #4)
> (From update of attachment 160591 [details] [review])
> This still has a behavioral change that I'm not sure was intentional.
> 
> In the current UI, "Alt-Tab, down" always selects the same window that would
> have been selected if you'd just done "Alt-Tab". With this patch, "Alt-Tab,
> down" selects the *next* window (just like "Alt-Tab, backtick" does).

Oh...when looking at a not-focused app, got it.  I was cofnused by this initially in testing since that behavior is what you want when your current focused app has multiple windows and you press down, right?

> If that was on purpose, then lgtm.

Nope not intentional, good catch.  Fixing.

(In reply to comment #5)
> Oh...when looking at a not-focused app, got it.

No, it's the same whether you're looking at the focused app or a not-focused app. 

In current git, pressing "down" is a no-op in terms of what window will get focused when you leave the switcher. Given a particular configuration of windows, either "Alt-Tab, release" or "Alt-Tab, down, release" (or "Alt-Tab, down, up, down, up, down, up, release") would bring you to the same window.

With this patch, pressing "down" *does* change what window would get focused. "Alt-Tab, release" would select one window, while "Alt-Tab, down, release" would have selected the *next* window in sequence, regardless of whether you were tabbing within the same app or between two apps.

Ah, actually what's been throwing me off a good bit is this:

 // If the next window of the current app is more-recently-used
 // than the first window of the next app, then select it.

I find this really confusing and unpredictable because about 80% of the time alt-tab goes between apps, except for this one special case where I'm trying to tab to my browser and wondering why the next window is selected.

Can this bit be removed now that we're by default raising all app windows?

Created attachment 160826 [details] [review]
Allow selection of individual windows

Per design discussion, change things back so that when choosing
an individual window, we raise only that window.  However
when we select an application, raise all windows.

A behavioral change required to clearly differentiate these
is that when the window thumbnail list is popped up, it no
longer has the first window selected by default.  Therefore
the user has to explicitly press the down arrow or use the
mouse to enter individual window selection mode.

This also removes the special case where we check if two windows
for an app have been interacted with more recently than the next
app.  That case should be obviated by the change to raise all
windows for an app.

This patch always selects the first window for Down, and removes the special case window handling.

(In reply to comment #8)
> 
> This also removes the special case where we check if two windows
> for an app have been interacted with more recently than the next
> app.  That case should be obviated by the change to raise all
> windows for an app.

There might be some arguments for removing the special case, but it has zero to do with raising all windows for an app.

The idea of the special case was to allow flipping between two windows of the same app with alt-Tab. Raising all windows of the application doesn't help with that.

(In reply to comment #10)
> (In reply to comment #8)
> > 
> > This also removes the special case where we check if two windows
> > for an app have been interacted with more recently than the next
> > app.  That case should be obviated by the change to raise all
> > windows for an app.
> 
> There might be some arguments for removing the special case, but it has zero to
> do with raising all windows for an app.

It does have *something* to do with it, because it's in conflict with the premise that alt-tab switches between apps.  If we pick a window in that case, we don't raise all of the windows.

Part of what I'm figuring out is weird to me is that if you have two apps each with two windows (say terminal and gedit) and you close a window on one of them then alt-tab, you can get dropped into the special case because of how we compare applications with the user time of their *open* windows.  So even though I interacted with one of gedit's windows more recently, the other window wasn't and so gets tossed way back in the order.

This is probably a ShellApp bug; we should just have a global user time for apps.

But I think you're right that my use of "obviates" was incorrect; raising all windows doesn't help you toggle between evolution and the composer window or whatever.

> The idea of the special case was to allow flipping between two windows of the
> same app with alt-Tab. Raising all windows of the application doesn't help with
> that.

My take is we need to get the Alt-` binding working for that.

OK, so how are we proceeding here? My instinct would be to land some patch that doesn't change the app vs. window behavior and treat removing that (if we want to) as a separate issue.

(In reply to comment #12)
> OK, so how are we proceeding here? My instinct would be to land some patch that
> doesn't change the app vs. window behavior and treat removing that (if we want
> to) as a separate issue.

You mean that doesn't remove the special case for same-app window interaction time?

(In reply to comment #13)
> (In reply to comment #12)
> > OK, so how are we proceeding here? My instinct would be to land some patch that
> > doesn't change the app vs. window behavior and treat removing that (if we want
> > to) as a separate issue.
> 
> You mean that doesn't remove the special case for same-app window interaction
> time?

Yes. That preserves the current behavior when the last-interacted-with window is from the same application.

Attachment 160826 [details] pushed as 1af392b - Allow selection of individual windows

Ok I readded the special casing.  I think we'll likely be iterating further on this, but it's a useful checkpoint to push now.