Bug 304860

Summary: Embedded Ark instances share the same "/DndExtract" object
Product: [Applications] ark Reporter: Diggory Hardy <kde2>
Component: generalAssignee: Raphael Kubo da Costa <rakuco>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 2.19   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 4.9.1
Sentry Crash Report:

Description Diggory Hardy 2012-08-09 14:20:59 UTC
I have an archive open in Ark embedded within rekonq ([1], in case it matters). If I try dragging one of the files in the archive to a dolphin window, I get something completely different extracted there! What actually gets created in the target directory is a file from another archive I viewed recently, complete with its directory tree from that archive.

I'm using KDE 4.9 from Chakra linux.
Comment 1 Raphael Kubo da Costa 2012-08-12 18:16:12 UTC
Can you reproduce this if you open the same archive in a separate Ark window? Does it happen only to this archive? Would it be possible to attach a file that presents this behavior?
Comment 2 Diggory Hardy 2012-08-20 15:32:01 UTC
No, it doesn't happen when dragging from an Ark window.

The following appears to reproduce:
1) open http://javadude.com/articles/antlr3xtut/antlr3xtut-part6.zip within rekonq
2) open http://switch.dl.sourceforge.net/project/sevenzip/7-Zip/9.20/7za920.zip within rekonq
3) open a dolphin window, create a temporary dir
4) drag some file from the first archive to the dolphin window
5) try dragging some file from the second archive — in some cases nothing happened (at least, before I did step 4), and in others it tries to extract the same file as extracted in step 4 again
Comment 3 Raphael Kubo da Costa 2012-08-21 02:22:17 UTC
Interesting. Confirming, as I was able to reproduce it with Konqueror as well.
Comment 4 Raphael Kubo da Costa 2012-08-21 03:26:36 UTC
Retitling in a more technically accurate way: the problem here is that part.cpp calls

    new DndExtractAdaptor(this);
    QDBusConnection::sessionBus().registerObject(QLatin1String( "/DndExtract" ), this);

in its constructor. This ends up registering the "/DndExtract" path in the D-Bus service started by Konqueror itself, so all Ark KParts end up sharing the same object, since QBusConnection::registerObject will not replace existing objects with the same path.
Comment 5 Raphael Kubo da Costa 2012-08-22 22:37:51 UTC
I have a working fix here, but unfortunately all programs which talk to Ark need to be fixed at the same time. There's a pending review request for Dolphin at <https://git.reviewboard.kde.org/r/106131/>.
Comment 6 Raphael Kubo da Costa 2012-08-26 04:47:11 UTC
Git commit c79d8db21a5d2c5686044cef497490d6c0f83b59 by Raphael Kubo da Costa.
Committed on 26/08/2012 at 06:09.
Pushed by rkcosta into branch 'KDE/4.9'.

Use a different D-Bus object path for each Ark::Part.

So far, all Ark::Part instances used the same D-Bus object path,
"/DndExtract", to receive drag'n'drop notifications.

This does not work correctly ever since we started supporting using Ark as
an embedded KPart (for previewing archived inside Konqueror or Rekonq, for
example). In this case, the object path is added to the embedder (the
`konqueror-5654' service, for example). If one previews multiple archives in
different tabs, multiple calls to QDBusConnection::registerObject() will be
made and only the first one will succeed, since we are always trying to
register the same path.

Fixing this involves touching separate parts of the code:

 o Use a different object path for each KPart instance, just like
   KateDocument or nsplugin do. We do this by keeping a static counter that
   is incremented each time a KPart is created and is part of the path name.

 o Use other, more specific mime types for the data we send when dragging
   out of Ark. So far we used "application/x-kde-dndextract" and passed the
   D-Bus service as its value. We now pass this value in the newly-created
   "application/x-kde-ark-dndextract-service" mime type, and the object
   path, which is now passed to ArchiveModel, in
   "application/x-kde-ark-dndextract-path".

 o Also use a more specific interface name in the XML file. While here,
   generate the .cpp and .h files with CMake instead of keeping these
   auto-generated files around for no purpose.

 o ArchiveModel::mimeTypes() has been adjusted to indicate the mime types we
   actually use on drag'n'drop. The previous list was mostly bogus, as we
   only use the mime types indicated above.

As it can be seen, applications which support drag'n'drop from Ark also need
to be adjusted to use the new D-Bus interface. Currently, the applications
are Dolphin, the Folderview plasmoid and a Stack Folder plasmoid in
playground. They are all going to be updated accordingly in tandem with this
commit to both KDE/4.9 and master. Users using SVN/git directly might
experience some trouble if they update one part but not the other, but there
is not much we can do in this case.
FIXED-IN:	4.9.1

M  +2    -1    part/CMakeLists.txt
M  +16   -33   part/archivemodel.cpp
M  +3    -1    part/archivemodel.h
D  +0    -41   part/dnddbusinterface.cpp
D  +0    -45   part/dnddbusinterface.h
M  +2    -2    part/dnddbusinterface.xml
M  +13   -6    part/part.cpp

http://commits.kde.org/ark/c79d8db21a5d2c5686044cef497490d6c0f83b59
Comment 7 Raphael Kubo da Costa 2012-08-26 04:48:02 UTC
Git commit 88e24922578b4391edba2d980a632958249d6eda by Raphael Kubo da Costa.
Committed on 24/08/2012 at 03:58.
Pushed by rkcosta into branch 'KDE/4.9'.

Adjust to Ark's drag'n'drop D-Bus interface changes.

Ark's drag'n'drop D-Bus interface needs to be changed: so far, the object
path was always /DndExtract, but this does not work if Ark is being used as
an embedded KPart (in Konqueror or Rekonq, for example), as all tabs will
end up calling QDBusConnection::registerObject() with the same path. Only
the first call will work, and the result is that dragging and dropping from
any tab previewing an archive with Ark will extract from the first archive
being previewed.

To fix that, applications that accept the application/x-kde-dndextract
mimetype should now be adjusted to check the
application/x-kde-ark-dndextract-service and
application/x-kde-ark-dndextract-path ones instead; the former contains the
same service information that used to be passed, while the latter tells
which object path should be talked to.

This is the Dolphin part of the change, which also needs to be made to
the folderview plasmoid.

REVIEW:		106131

M  +7    -4    dolphin/src/views/draganddrophelper.cpp

http://commits.kde.org/kde-baseapps/88e24922578b4391edba2d980a632958249d6eda
Comment 8 Raphael Kubo da Costa 2012-08-26 04:48:02 UTC
Git commit 396c2c3fcffc0536fc5c503202a8636c9491ec0c by Raphael Kubo da Costa.
Committed on 25/08/2012 at 03:44.
Pushed by rkcosta into branch 'KDE/4.9'.

Adjust to Ark's drag'n'drop D-Bus interface changes.

Ark's drag'n'drop D-Bus interface needs to be changed: so far, the object
path was always /DndExtract, but this does not work if Ark is being used as
an embedded KPart (in Konqueror or Rekonq, for example), as all tabs will
end up calling QDBusConnection::registerObject() with the same path. Only
the first call will work, and the result is that dragging and dropping from
any tab previewing an archive with Ark will extract from the first archive
being previewed.

To fix that, applications that accept the application/x-kde-dndextract
mimetype should now be adjusted to check the
application/x-kde-ark-dndextract-service and
application/x-kde-ark-dndextract-path ones instead; the former contains the
same service information that used to be passed, while the latter tells
which object path should be talked to.

This is the folderview part of the change, modeled after the changes made to
Dolphin.

M  +18   -15   plasma/applets/folderview/iconview.cpp

http://commits.kde.org/kde-baseapps/396c2c3fcffc0536fc5c503202a8636c9491ec0c