Unity

Merge lp://qastaging/~3v1n0/unity/hud-invalid-icon-fix into lp://qastaging/unity

hud-invalid-icon-fix
Merge into trunk

Proposed by Marco Trevisan (Treviño) on 2012-04-01

Status:

Merged

Approved by:

Thomi Richards on 2012-04-03

Approved revision:

no longer in the source branch.

Merged at revision:

2212

Proposed branch:

lp://qastaging/~3v1n0/unity/hud-invalid-icon-fix

Merge into:

lp://qastaging/unity

Diff against target:

510 lines (+161/-79)

10 files modified

plugins/unityshell/src/HudController.cpp (+45/-5)
plugins/unityshell/src/HudIcon.cpp (+5/-6)
plugins/unityshell/src/HudLauncherIcon.cpp (+7/-4)
tests/autopilot/autopilot/emulators/unity/hud.py (+15/-2)
tests/autopilot/autopilot/emulators/unity/icons.py (+3/-1)
tests/autopilot/autopilot/keybindings.py (+1/-0)
tests/autopilot/autopilot/tests/__init__.py (+6/-0)
tests/autopilot/autopilot/tests/test_hud.py (+50/-31)
tests/autopilot/autopilot/tests/test_showdesktop.py (+18/-14)
tests/autopilot/autopilot/tests/test_switcher.py (+11/-16)

To merge this branch:

bzr merge lp://qastaging/~3v1n0/unity/hud-invalid-icon-fix

Related bugs:

Bug #931548: Show default HUD icon for Desktop (nautilus) menus	Medium	Fix Released
Bug #932371: Hud shows wrong application icon when activated with the dash showing	Medium	Fix Released
Bug #961161: HUD remembers icon of last used app	Medium	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Thomi Richards (community)		2012-04-01	Approve on 2012-04-03
John Lea (community)	design		Approve on 2012-04-02
Review via email: mp+100318@code.qastaging.launchpad.net

Commit message

HudController: use the top-most valid window in the stack to fetch the icon, if an invalid window is focused

Description of the change

HudController: use the top-most valid window in the stack to fetch the icon, if an invalid window is focused

When the dash or any other unity window is focused, the HUD isn't able to get the icon from the currently focused window (this manifests with bug #932371), so we need to fallback to the top-most valid window in the stack.

Getting the list of windows by stack order from BAMF, we can easily get the top-most application and then retrieve its icon. Thanks to this switching from HUD to Dash and the other way around, works as expected.

In the case that no valid window is focused, we fallback to the Ubuntu icon.

This includes AP tests.

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2012-04-01:

Hi,

Great branch - a few small issues with the AP test though:

99 + self.keybinding("dash/reveal", 0.1)
100 + sleep(1)

Please use self.dash.ensure_visible() for this - it's safer, since it doesn't require you to sleep() after calling it.

105 + if self.hud.is_locked_launcher:
106 + hud_launcher_icon = self.get_hud_launcher_icon()
107 + self.assertThat(hud_launcher_icon.icon_name, Equals(calc.icon))
108 + else:
109 + hud_embedded_icon = self.hud.get_embedded_icon()
110 + self.assertThat(hud_embedded_icon.icon_name, Equals(calc.icon))

This seems to be something we're going to be doing a lot, so please just add a property to the Hud class that gets you the correct icon in all situations. THen these 6 lines become one:

self.assertThat(self.hud.icon.name, Equals(calc.icon))

... or something similar.

114 + self.start_app("Calculator")
115 + calctools = self.get_app_instances("Calculator")
116 + self.assertThat(len(calctools), GreaterThan(0))
117 + calc = calctools[0]
118 + self.assertTrue(calc.is_active)

This is ugly - and it's my fault. Please could you patch self.start_app(...) so it returns the BamfApplication instance? It shouldn't be hard to do, since we launch the application in the Bamf Emulator. THen you can replace all that code with this:

calc = self.start_app("Calculator")

Cheers,

review: Needs Fixing

Revision history for this message

John Lea (johnlea) wrote on 2012-04-02:

Thx, yes the bfb should be used.

review: Approve (design)

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-04-03:

> Please use self.dash.ensure_visible() for this - it's safer, since it doesn't
> require you to sleep() after calling it.

Done.

> 105 + if self.hud.is_locked_launcher:
> 106 + hud_launcher_icon = self.get_hud_launcher_icon()
> 107 + self.assertThat(hud_launcher_icon.icon_name,
> Equals(calc.icon))
> 108 + else:
> 109 + hud_embedded_icon = self.hud.get_embedded_icon()
> 110 + self.assertThat(hud_embedded_icon.icon_name,
> Equals(calc.icon))
>
> This seems to be something we're going to be doing a lot, so please just add a
> property to the Hud class that gets you the correct icon in all situations.
> THen these 6 lines become one:
>
> self.assertThat(self.hud.icon.name, Equals(calc.icon))

Done.

> 114 + self.start_app("Calculator")
> 115 + calctools = self.get_app_instances("Calculator")
> 116 + self.assertThat(len(calctools), GreaterThan(0))
> 117 + calc = calctools[0]
> 118 + self.assertTrue(calc.is_active)
>
>
> This is ugly - and it's my fault. Please could you patch self.start_app(...)
> so it returns the BamfApplication instance? It shouldn't be hard to do, since
> we launch the application in the Bamf Emulator. THen you can replace all that
> code with this:
>
> calc = self.start_app("Calculator")