[Bugs] #4276 sugar-toolkit-gtk3 HIGH: Journal: .xo file in Documents or pendrive show a wrong icon

[Sugar Labs Bugs]

#4276: Journal: .xo file in Documents or pendrive show a wrong icon
-----------------------------------+----------------------------------------
    Reporter:  godiard             |          Owner:  erikos        
        Type:  defect              |         Status:  new           
    Priority:  High                |      Milestone:  0.98          
   Component:  sugar-toolkit-gtk3  |        Version:  Unspecified   
    Severity:  Unspecified         |       Keywords:  regression, r?
Distribution:  Fedora              |   Status_field:  Unconfirmed   
-----------------------------------+----------------------------------------
Changes (by manuq):

  * keywords:  regression => regression, r?


Comment:

 So the pygi warning [http://bugs.sugarlabs.org/attachment/ticket/4276
 /shell-wrong-icon.log#L285 in the log here] is because the journal model
 for 'file-name', which should be a 'str' with the path of the icon, is
 getting a sugar3.util.TempFilePath instead.  That class actually inherits
 'str', but seems that pygobject doesn't like it.

 For a quick workaround I casted the result of get_icon as 'str':

 {{{
 --- a/src/jarabe/journal/misc.py
 +++ b/src/jarabe/journal/misc.py
 @@ -74,7 +74,7 @@ def get_icon_name(metadata):
          if file_path is not None and os.path.exists(file_path):
              try:
                  bundle = ActivityBundle(file_path)
 -                file_name = bundle.get_icon()
 +                file_name = str(bundle.get_icon())
              except Exception:
                  logging.exception('Could not read bundle')
 }}}

 And then I come to the next issue: as Ajay said, the temporal file is
 deleted before the CellRendererIcon reads it.

 {{{
 1355603345.296226 DEBUG root: TempFilePath created '/tmp/activity-
 webu590GL.svg'
 1355603345.300510 DEBUG root: TempFilePath deleted '/tmp/activity-
 webu590GL.svg'
 1355603345.301241 DEBUG root: MANUQ get_icon_name got '/tmp/activity-
 webu590GL.svg' of type <type 'str'>
 Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/sugar3/graphics/icon.py", line
 952, in do_render
     surface = self._buffer.get_surface()
   File "/usr/lib/python2.7/site-packages/sugar3/graphics/icon.py", line
 261, in get_surface
     handle = self._load_svg(icon_info.file_name)
   File "/usr/lib/python2.7/site-packages/sugar3/graphics/icon.py", line
 121, in _load_svg
     return self._loader.load(file_name, entities, self.cache)
   File "/usr/lib/python2.7/site-packages/sugar3/graphics/icon.py", line
 51, in load
     icon_file = open(file_name, 'r')
 IOError: [Errno 2] No existe el fichero o el directorio: '/tmp/activity-
 webu590GL.svg'
 }}}

 What happens is as we don't maintain any reference to the TempFilePath,
 its __del__ method deletes the temporal file.

 As a comment in the get_icon method of ActivityBundle say, ideally we
 should return the icon data.  But that implies a major change in the
 Journal model and views, and in the CellRendererIcon class.

 So I propose returning the temporal file path directly, instead of
 wrapping it in a TempFilePath.  This has one disadventage, the temporal
 files are left in /tmp and that directory can get crowded.  But that is
 not harmful

 The attached patch does so.

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/4276#comment:9>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system