Unity

Merge lp://qastaging/~mhr3/unity/fix-856205 into lp://qastaging/unity

  1. fix-856205
  2. Merge into trunk
Proposed by Michal Hruby
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 1746
Proposed branch: lp://qastaging/~mhr3/unity/fix-856205
Merge into: lp://qastaging/unity
Diff against target: 124 lines (+58/-1)
3 files modified
manual-tests/Dash.txt (+13/-0)
plugins/unityshell/src/DashView.cpp (+39/-1)
plugins/unityshell/src/DashView.h (+6/-0)
To merge this branch: bzr merge lp://qastaging/~mhr3/unity/fix-856205
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Mikkel Kamstrup Erlandsen (community) Approve
Review via email: mp+83263@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2011-11-16.

Description of the change

Fixes bug #856205 by delaying activation of the first item in the model. Note that this requires also change to lenses (because currently they emit the SearchFinished signal before updating the model).

As didrocks informed me, this can't just go to stable branch without going to trunk, although I have much nicer fix in mind once we change some libunity internals.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

I tested this quite thoroughly and it seems to work well. Code looks good too. Although there are some nitpicks:

 i) in ResetSearchState() Why reinterpret_cast and not static_cast?

 ii) in ResetSearchState() should we not g_source_remove (searching_timeout_id_); if the id != 0?

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

Inside the unity code, can we please stay away from glib types?

Just use "unsigned" instead of "guint".

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

@Tim - that seems to be begging for trouble? This begs for 32/64 bit issues and nasty bool/gboolean issues both of which we've seen before.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Like the changes. Re-approved! :-)

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

Due to lack of infrastructure for testing the timing issues here, I'm happy with a manual test.

review: Approve

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.