Code review comment for lp:~unity-team/unity/places-fixes-2010-09-22

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

CODE REVIEW:
unity-private/places/places-place-home.vala:
 You are using ObjectRegistry to look up the view. This seems like a pretty big hack, abusing the testing infrastructure..?

FUNCTIONAL REVIEW:

Bug #607821: Search Field does not behave as specified when the search fails
 - Fix confirmed

Bug #607829: Clicking a Place icon while viewing the same place in the Dash should return to the Home screen of that place and clear the search
 - Fix confirmed

Bug #612562: URI activation in global view
 - Fix confirmed

Bug #634364: unity crashed with SIGSEGV in unity_places_place_entry_remote_set_active()
 - I could reproduce this now. See http://paste.ubuntu.com/498597/. I can't exactly figure out where to insert the null checks. And frankly I am more concerned why u-p-a fails to come up...

Bug #637136: launchers should act like if the application was not focussed in the place views
 - Fix confirmed

Bug #638402: clicking on a category from CoFs does not directly take you to the desired category
 - Fix confirmed

Bug #638857: dash initial icons states
 - Fix confirmed

Bug #641554: cropping an image in shotwell crashes unity
 - Not tested

Bug #641669: Launcher loads 48 px icons instead of 32 px
 - Fix confirmed

Bug #641894: Clicking on the "Files & Folders" icon while a folder is already displayed in the overlay view causes unity to crash and re-start
 - I get a crash here still. 100% reproducible.

Bug #642669: mutter crashes when closing pop-up dialog
 - I still get very rare crashes when I close the 'bzr gcommit' dialog, in maximized state, when clicking the commit button

Bug #643627: mutter crashed with SIGSEGV in unity_places_place_entry_get_parent()
 -

Bug #644215: "Applications" and "Files & Folders" tooltips are not translatable
 - Fix confirmed

CONCLUSION
 * Can you recheck the fix for bug #641894?
 * Bug #634364 scares me a bit... I don't have a ready fix right now.

« Back to merge proposal