#5106 HIGH Update.: Attaching a file to gmail msg caused the journal to crash

[Zarro Boogs per Child]

#5106: Attaching a file to gmail msg caused the journal to crash
-------------------------------+--------------------------------------------
  Reporter:  kimquirk          |       Owner:  tomeu   
      Type:  defect            |      Status:  new     
  Priority:  high              |   Milestone:  Update.1
 Component:  journal-activity  |     Version:          
Resolution:                    |    Keywords:  review? 
  Verified:  0                 |  
-------------------------------+--------------------------------------------

Comment(by rwh):

 In general I think this is the good approach, so a review+. However, I'd
 like to hear comment on my DBus comment so I'll leave the review? for now.

 '''collapsedentry.py'''

 {{{
 if not self._allow_resume:
    logging.debug('self._frame.set_child_visible(%r, %r)' %
 (self._resume_button, False))
 }}}

 Do we need this debug msg? Marco would say: 80 cols...

 {{{
 def _create_icon(self):
 }}}
 Why do we connect_after, and to the release-event instead of clicked? This
 was already there, so I assume it is correct.

 '''journalactivity.py'''
 {{{
     @dbus.service.method(J_DBUS_INTERFACE, in_signature='i',
 out_signature='s')
     def ChooseObject(self, parent_xid):
 }}}

 Wouldn't it be nicer design-wise to make this function blocking and return
 the object_id in combination with calling it asynchronously? Maybe even a
 race here because you connect to the DBus signal after calling the
 ChooseObject function. You could connect before and set self._chooser_id
 to None before calling _self.chooser_id = journal.ChooseObject().

 '''objectchooser.py'''
         height = (self.get_screen().get_height() / style.GRID_CELL_SIZE -
 1) * style.GRID_CELL_SIZE

 Marco 80 cols :)

 {{{
     def __realize_cb(self, chooser, parent):
         self.window.set_transient_for(parent)
 }}}

 Can't you just do this in the constructor? That's what I did with the
 unfullscreen button at least.

 Use of gtk.RESPONSE_DELETE_EVENT sounds as if dangerous stuff is happeing
 :) I think gtk.RESPONSE_REJECT or gtk.RESPONSE_CANCEL is more appropriate.

 '''lib/sugar/graphics/objectchooser.py'''
 Looks much simpler then the previous version I think!

-- 
Ticket URL: <http://dev.laptop.org/ticket/5106#comment:10>
One Laptop Per Child <http://dev.laptop.org>
OLPC bug tracking system