Journal: .xo file in Documents or pendrive show a wrong icon

[godiard]

TestCase

Copy a .xo file in the Documents directory

Go to the Journal and show the directory

The icon show is not the real icon, but one from other activity.

Tested in os15, xo-4

[erikos]

I copied edit-8.xo from the Journal into the Documents folder using the copy to option in the Palette. I had been downloading the activity before. There was already Write-83.1.xo in the Documents folder.

Now the Documents folder view shows:

• an entry "edit-8" with the edit icon (in the Journal the entry is named "edit-8.xo")

• another entry "Write-83.1.xo" with the edit icon

If I go to the details view of "Write-83.1.xo" the icon is the one from the Write activity. If I go to the details view of "edit-8" the edit activity icon is shown.

Copying other entries from the Journal to the Documents folder do show the correct icon.

[erikos]

Furthermore:

• the right click Palette of the "Write-83.1.xo" does show the correct icon.

• copying another activity I downloaded, the view stays the same, only the "Write-83.1.xo" entry is wrong.

[erikos]

I downgraded the DS for #4276: ​http://arm.koji.fedoraproject.org/koji/buildinfo?buildID=96531

I think the issue is gone with this.

[manuq]

Replying to erikos:

I downgraded the DS for #4276: ​http://arm.koji.fedoraproject.org/koji/buildinfo?buildID=96531

I think the issue is gone with this.

I downgraded the DS too, for an .xo file in a stick I can still see the document-like icon instead of the activity icon (which does display in the details view and palette menu).

[ajay_garg]

Patch at ​http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041136.html

[ajay_garg]

An accompanying sugar-patch has been floated at ​http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041181.html

This patch removes the icon-files directory on every sugar-reboot, to prevent accumulation of any spurious files.

Thanks to James Cameron <quozl@…>, for the patch-idea.

[ajay_garg]

Version-2 of the sugar-patch at ​http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041184.html

[manuq]

Lots of guessing but no one provided a log :)

[manuq]

• ​log line 284, the tempfile for the svg icon is created

• ​log line 285 and 286, we have a warning from pygi - unable to set property file-name' of type gchararray' from value of type `PyObject'

• ​log line 287, the tempfile for the svg icon is deleted

[manuq]

Candidate patch.

[manuq]

So the pygi warning ​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.

[manuq]

Replying to manuq:

the temporal files are left in /tmp and that directory can get crowded. But that is not harmful

Because /tmp is mounted as a tmpfs, so it is cleaned by the system after each reboot or when needed.

[humitos]

The patch looks good as a quick fix. But as you said, we have to improve this in the future. I tested it on XO-4 13.1.0 build 19 and it worked.

Check #1175 for some history on a similar issue.

[manuq]

Pushed b6152b02 .

[greenfeld]

I see the correct icon for several xo files on a USB stick in 13.1.0 os24/Sugar 0.98.3.

[dnarvaez]

Milestone 0.98 deleted