Merge lp:~unity-team/unity/3v1n0-quick-alt+tab-fixes into lp:unity
| Status: | Merged |
|---|---|
| Approved by: | Tim Penhey on 2012-04-16 |
| Approved revision: | 1822 |
| Merged at revision: | 2282 |
| Proposed branch: | lp:~unity-team/unity/3v1n0-quick-alt+tab-fixes |
| Merge into: | lp:unity |
| Diff against target: |
815 lines (+336/-97) 16 files modified
manual-tests/Switcher.txt (+1/-1) plugins/unityshell/src/BamfLauncherIcon.cpp (+45/-30) plugins/unityshell/src/Launcher.cpp (+3/-1) plugins/unityshell/src/PluginAdapter.cpp (+91/-47) plugins/unityshell/src/PluginAdapter.h (+4/-1) plugins/unityshell/src/SwitcherController.cpp (+1/-0) plugins/unityshell/src/UScreen.cpp (+9/-3) plugins/unityshell/src/UScreen.h (+1/-0) plugins/unityshell/src/WindowManager.cpp (+11/-1) plugins/unityshell/src/WindowManager.h (+5/-2) plugins/unityshell/src/unityshell.cpp (+8/-5) tests/autopilot/autopilot/emulators/bamf.py (+11/-2) tests/autopilot/autopilot/emulators/unity/switcher.py (+3/-0) tests/autopilot/autopilot/tests/__init__.py (+12/-0) tests/autopilot/autopilot/tests/test_launcher.py (+65/-0) tests/autopilot/autopilot/tests/test_switcher.py (+66/-4) |
| To merge this branch: | bzr merge lp:~unity-team/unity/3v1n0-quick-alt+tab-fixes |
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Thomi Richards (community) | quality | Approve on 2012-04-16 | |
| Tim Penhey (community) | 2012-04-15 | Approve on 2012-04-16 | |
| Sam Spilsbury | 2012-04-15 | Pending | |
| Alex Launi | quality | 2012-04-15 | Pending |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2012-04-05.
Commit Message
Fix the behaviour of alt-tab and clicking on the launcher icon to just raise the most recently used window, not all for that app.
BamfLauncherIcon's activation related code has been updated to be more multimonitor aware (for free I've fixed also some bugs that caused the windows not to be put in spread mode in multi-monitor), and to support both the "Launcher only on Primary Monitor" and "Launcher on all monitors" options.
PluginAdapter's FocusWindowGroup method has been updated to optionally only unminimize / raise and activate only the top window. This code would have been more optimized using a reverse iterator to fetch the top_window, but not to change the whole logic and to allow to keep the previous behavior (that initially we wanted for "long alt+tab") without duplicating code, I've just hacked that.
Implemented also GetWindowMonitor to workaround the mismatch we had with the compiz' window-
Screencast of the fixed version: http://
Description of the Change
== Problem ==
Launcher, Alt-Tab - clicking on launcher item or selecting a app in Alt-Tab raises all app windows, not just most recently focused.
== Fix ==
BamfLauncherIcon's activation related code has been updated to be more multimonitor aware (for free I've fixed also some bugs that caused the windows not to be put in spread mode in multi-monitor), and to support both the "Launcher only on Primary Monitor" and "Launcher on all monitors" options.
PluginAdapter's FocusWindowGroup method has been updated to optionally only unminimize / raise and activate only the top window. This code would have been more optimized using a reverse iterator to fetch the top_window, but not to change the whole logic and to allow to keep the previous behavior (that initially we wanted for "long alt+tab") without duplicating code, I've just hacked that.
Implemented also GetWindowMonitor to workaround the mismatch we had with the compiz' window-
Screencast of the fixed version: http://
== Test ==
There are autopilot test for both bugs.
Thanks to Brandon for taking care of merging the old lp:~3v1n0/unity/quick-alt+tab-fixes with trunk and for the AP tests that I used as my base.
UNBLOCK
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
| Brandon Schaefer (brandontschaefer) wrote : | # |
I ran all the autopilot test that I thought these changes would effect.
Launcher Autopilot tests:
Ran 43 tests in 296.560s
OK
Switcher Autopilot tests:
Ran 21 tests in 187.666s
OK
Dash Autopilot tests:
Ran 32 tests in 225.549s
OK
| Sam Spilsbury (smspillaz) wrote : | # |
Tests OK, no iconic window regressions (which is what I was a little concerned about)
| Tim Penhey (thumper) wrote : | # |
Clicking on the launcher to reveal only the top most one works.
However the quick alt-tab one does not work.
If I have two terminal windows and one gedit. Focus gedit and quick alt-tab, it raises both terminals.
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Probably it is caused by the fact that now we use a very small delay for the Alt+Tab.
A workaround could be to use another time reference to consider an Alt+Tab quick, more than the only window visibility.
I mean, even if the window is shown for 100ms or less, the Alt+Tab is surely a quick one (as there's no time to look to the content of the window).
| Brandon Schaefer (brandontschaefer) wrote : | # |
I just ended up making a timer called quick_tab_timer_, which will go for 200ms if you hold on to alt for any longer it makes quick_tab = false. Seems like a nice and customizable way.
| Alex Launi (alexlauni) wrote : | # |
It shouldn't be too difficult to automate the manual tests. Please do not merge this with the manual tests.
| Brandon Schaefer (brandontschaefer) wrote : | # |
@Alex
Sorry about having the manual test in there. I was trying to get this branch in 5.10 and was rushing to get it done. Never a good sign when it comes to quality. There are now autopilot test for each bug!
| Brandon Schaefer (brandontschaefer) wrote : | # |
So I guess diff continutes to complain about there not being a new line...I even took the trunk version of Switcher.txt and replaced it and it still gives that problem...
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
I've just checked this code again, there's a problem with the minimized windows when using the launcher icon.
Don't worry about that Brandon, I'll take care of that ;)
| Brandon Schaefer (brandontschaefer) wrote : | # |
Yeah, I just saw that. At lease we know what design wants now :). Thank you!
- 1817. By Marco Trevisan (Treviño) on 2012-04-15
-
PluginAdapter: avoid to raise and unminimize top_window if already done
| Brandon Schaefer (brandontschaefer) wrote : | # |
Awesome work! Confirmed everything I could with only 1 monitor, bot AP test pass as well as doing a lot of manual testing :)
- 1818. By Tim Penhey on 2012-04-15
-
Tweaks to the test.
- 1819. By Tim Penhey on 2012-04-15
-
More AP test tweaks.
- 1820. By Tim Penhey on 2012-04-16
-
Assert that the visible window stack is what we expect.
- 1821. By Tim Penhey on 2012-04-16
-
Add window stack assertions to the switcher tests.
- 1822. By Tim Penhey on 2012-04-16
-
Test the alt-tab appears on the monitor that has window focus.
| Tim Penhey (thumper) wrote : | # |
The code looks fine, and I've tested the behaviour and tweaked the AP tests. Thomi will review the tests.


30 - if ((mapped && wm->IsWindowMap ped(xid) ) || !mapped) ped(xid) && !bamf_window_ get_transient( BAMF_WINDOW( view))) || !mapped)
31 + if ((mapped && wm->IsWindowMap
32 {
About this, I think I included that change mostly for testing purposes, I'm not sure it's actually wanted so move it out.
I didn't work too much on my branch lately since I didn't know if that was still the wanted desig, but if now it is, I'll be happy to get this merged :)