Unity

Merge lp://qastaging/~townsend/unity/lp1070715 into lp://qastaging/unity

  1. lp1070715
  2. Merge into trunk
Proposed by Christopher Townsend
Status: Merged
Approved by: Christopher Townsend
Approved revision: no longer in the source branch.
Merged at revision: 3158
Proposed branch: lp://qastaging/~townsend/unity/lp1070715
Merge into: lp://qastaging/unity
Diff against target: 52 lines (+28/-1)
2 files modified
launcher/SwitcherModel.cpp (+8/-1)
tests/test_switcher_model.cpp (+20/-0)
To merge this branch: bzr merge lp://qastaging/~townsend/unity/lp1070715
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+148036@code.qastaging.launchpad.net

Commit message

Webapps cause two active applications in some cases, so added a check to only pick up the first active app which is the web browser. Fixes: https://bugs.launchpad.net/bugs/1070715

Description of the change

When using Webapps, two icons are considered Active. The logic to find the last active application did not take this into account. Since a Webapp is always the last icon, this change picks up the first active app in the SwitcherModel constructor.

Also added a unit test to simulate the condition of a Webapp being active.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

This seems to work, and I don't see regressions (more important thing here! :)).

However, at this point you can remove the last_active_application_ var at all from the model.

Also, would be nice to have some test (even if this maybe somewhat already tested). I think there are some in test_switcher_model.cpp that should inspire you.

Revision history for this message
Christopher Townsend (townsend) wrote :

Good point about last_active_application_.

I'll also see about adding a test for this.

Revision history for this message
Christopher Townsend (townsend) wrote :

Seems my latest fix is not good with very recent versions of the Unity code base. Back to the drawing board.

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

This fixes the sorting order problem, but like Chris mentioned rev 3153 caused this problem back up again. Merging this to get the sorting problem fixed and then to work on fixing the focus problem.

LGTM.

review: Approve
Revision history for this message
Christopher Townsend (townsend) wrote :

Actually, the fix is still needed. However, http://bazaar.launchpad.net/~unity-team/unity/trunk/revision/3153 has done something to cause a window focusing issue.

We're going to work to get this merged in and then look at the window focusing issue.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.