Bug 91410

Hi!

Em Sex 15 Out 2004 15:41, Wilbert Berendsen escreveu:
> In Ark I selected Action->Unpack (why is this option not in the File
> menu?). 

You might ask kde-usability about this.

> Apparently Ark backgrounds (why?) causing KMail to delete the attachment
> temp file.

Ark is a KUniqueApplication, and forking is a consequence of that.

> Don't know if I should file a wish for Ark for not backgrounding, or for
> KMail to leave tempfiles for e.g. a few minutes before deleting them (after
> the attachment viewer process exited).

I think KMail should not rely on the process ending for detecting when to 
delete temporary files. Some programs will "end" prematurely (like 
KUniqueApplications), some other might not exit when the file is closed 
(Kate, for instance).

On Friday 15 October 2004 22:30, Henrique Pinto wrote:
> I think KMail should not rely on the process ending for detecting when to 
> delete temporary files. Some programs will "end" prematurely (like 
> KUniqueApplications), some other might not exit when the file is closed 
> (Kate, for instance).

Problem is, if we don't do that, we can *never* delete temp files.

I guess we need to standardize on a --tempfile option to let the application (ark, here)
know that it should delete the file when it's done with it. Difficult problem [we can't know
if the app supports it, unless we e.g. add a flag to a .desktop file].

Previously kmail would delete the tempfile when switching to another mail - which
led to all sorts of race conditions (#39537). Now we use KRun's "tempfile" flag, which
means "delete the tempfile when the application exits". But this needs special support
for KUniqueApplication, maybe. Even if that simply means "noticing in the .desktop 
file that this is a KUniqueApplication, and not deleting the tempfile in that case".

Neither solution will work for non-KDE applications that have kuniqueapp-like behavior
and no --tempfile option or similar, of course, but we can't solve world hunger either :/

It's all a bit hacky.

I think it is good that KMail deletes the file when the handling process exits, but is it not a nice idea to just delete it a bit later, with some timer? E.g. just keep it for 10 minutes when the viewer process exits very quickly (a sign of backgrounding).

I think this is better and less hacky than the --tempfile idea.

> It's all a bit hacky.
> 
> I think it is good that KMail deletes the file when the handling process exits, but is it not a nice idea to just delete it a bit later, with some timer? E.g. just keep it for 10 minutes when the viewer process exits very quickly (a sign of backgrounding).
> 
> I think this is better and less hacky than the --tempfile idea.

That's an idea. It means keeping a running (but sleeping) kioexec process for 10 minutes though.
Well maybe 10 minutes is a bit too much anyway :)

On Friday 15 October 2004 22:52, David Faure wrote:
> On Friday 15 October 2004 22:30, Henrique Pinto wrote:
> > I think KMail should not rely on the process ending for detecting when to
> > delete temporary files. Some programs will "end" prematurely (like
> > KUniqueApplications), some other might not exit when the file is closed
> > (Kate, for instance).
>
> Problem is, if we don't do that, we can *never* delete temp files.
>
> I guess we need to standardize on a --tempfile option to let the
> application (ark, here) know that it should delete the file when it's done
> with it. Difficult problem [we can't know if the app supports it, unless we
> e.g. add a flag to a .desktop file].
>
> Previously kmail would delete the tempfile when switching to another mail -
> which led to all sorts of race conditions (#39537). Now we use KRun's
> "tempfile" flag, which means "delete the tempfile when the application
> exits". But this needs special support for KUniqueApplication, maybe. Even
> if that simply means "noticing in the .desktop file that this is a
> KUniqueApplication, and not deleting the tempfile in that case".

But *who* does delete the tempfile in that case?

> Neither solution will work for non-KDE applications that have
> kuniqueapp-like behavior and no --tempfile option or similar, of course,
> but we can't solve world hunger either :/

KUniqueApplication gives a well-defined window in which the application should 
have processed the file: As long as it handles the file before returning from 
newInstance() all will be well. I understand that that may be 
difficult/impossible in certain situations though... take for example a html 
page rendered in konqueror.

So I agree that if we have a --tempfile command line option, we can transfer 
the responsibility of deleting the file to the application, the application 
should then indicate in its .desktop file that it indeed wants to delete the 
file itself. I don't think that's something that we can handle 
automatically... the application developer will need to 
1) Enable the --tempfile option
2) Add the flag to the .desktop file (X-KDE-DeleteTempfile ?)
3) Actually make sure to delete files passed with --tempfile

What we need to do is then:
1) Fix KRun to check for the flag in the .desktop file
2) Add support for the standard --tempfile option (In e.g. KCmdLineOptions, 
KApplication or KUniqueApplication) Mostly a matter of a one-line 
KCmdLineArgs::addCmdLineOptions() call.

Cheers,
Waldo

CVS commit by faure: 

Added KCmdLineArgs::addTempFileOption(), and mention it in the KUniqueApplication docs
together with the .desktop file flag.
CCBUG: 91410, 62555


  M +21 -0     kcmdlineargs.cpp   1.95 [POSSIBLY UNSAFE: qDebug]
  M +12 -0     kcmdlineargs.h   1.67
  M +7 -0      kuniqueapplication.h   1.35


--- kdelibs/kdecore/kuniqueapplication.h  #1.34:1.35
@@ -36,4 +36,11 @@ class KUniqueApplicationPrivate;
  * the information to the first instance and then quit.
  *
+ * The .desktop file for the application should state X-DCOP-ServiceType=Unique,
+ * see kapplication.h
+ *
+ * If your application is used to open files, it should also support the --tempfile
+ * option (see KCmdLineArgs::addTempFileOption()), to delete tempfiles after use.
+ * Add X-KDE-HasTempFileOption=true to the .desktop file to indicate this.
+ *
  * @see KApplication DCOPObject
  * @author Preston Brown <pbrown@kde.org>

--- kdelibs/kdecore/kcmdlineargs.h  #1.66:1.67
@@ -535,4 +535,16 @@ public:
   static void loadAppArgs( QDataStream &);
 
+  /**
+   * Add standard option --tempfile
+   */
+  static void addTempFileOption();
+
+  // this avoids having to know the "id" used by addTempFileOption
+  // but this approach doesn't scale well, we can't have 50 standard options here...
+  /**
+   * @return true if --tempfile was set
+   */
+  static bool isTempFileSet();
+
 protected:
   /**

--- kdelibs/kdecore/kcmdlineargs.cpp  #1.94:1.95
@@ -1242,2 +1242,23 @@ KCmdLineArgs::addArgument(const char *ar
    parsedArgList->append(argument);
 }
+
+static const KCmdLineOptions kde_tempfile_option[] =
+{
+   { "tempfile",       I18N_NOOP("The files/urls opened by the application will be deleted after use"), 0},
+   KCmdLineLastOption
+};
+
+void
+KCmdLineArgs::addTempFileOption()
+{
+    qDebug( "addTempFileOption" );
+    KCmdLineArgs::addCmdLineOptions( kde_tempfile_option, "KDE-tempfile", "kde-tempfile" );
+}
+
+bool KCmdLineArgs::isTempFileSet()
+{
+    KCmdLineArgs* args = KCmdLineArgs::parsedArgs( "kde-tempfile" );
+    if ( args )
+        return args->isSet( "tempfile" );
+    return false;
+}

CVS commit by faure: 

Support for --tempfile in kfmclient and konqueror, to delete a tempfile after viewing it.
Requires passing that bool around quite a lot - from kfmclient to konqueror, and internally in konqueror.
CCBUG: 91410, 62555


  M +11 -0     KonqMainWindowIface.cc   1.24
  M +3 -0      KonqMainWindowIface.h   1.13
  M +8 -14     KonquerorIface.cc   1.41
  M +13 -5     KonquerorIface.h   1.29
  M +1 -0      kfmclient.desktop   1.90
  M +1 -0      kfmclient_html.desktop   1.52
  M +1 -0      kfmclient_war.desktop   1.27
  M +7 -5      konq_main.cc   1.143
  M +4 -3      konq_mainwindow.cc   1.1369
  M +2 -2      konq_mainwindow.h   1.442
  M +9 -7      konq_misc.cc   1.58
  M +5 -3      konq_misc.h   1.19
  M +10 -4     konq_openurlrequest.h   1.12
  M +24 -2     konq_view.cc   1.357
  M +8 -3      konq_view.h   1.171
  M +1 -0      konqueror.desktop   1.268
  M +16 -10    client/kfmclient.cc   1.119
  M +2 -2      client/kfmclient.h   1.28

CVS commit by faure: 

Implemented --tempfile in ark, to fix kmail bug #91410.
The other KUniqueApplications which open files seem to be:
korganizer, juk and quanta.

Can the developer of those apps add --tempfile support? :)
(see kuniqueapplication.h for details)

CCMAIL: reinhold@kainhofer.com, wheeler@kde.org, amantia@kde.org, 91410@bugs.kde.org


  M +1 -0      ark.desktop   1.259
  M +2 -1      arkapp.cpp   1.37
  M +8 -5      arktoplevelwindow.cpp   1.27
  M +1 -1      arktoplevelwindow.h   1.12
  M +38 -26    arkwidget.cpp   1.93
  M +12 -10    arkwidget.h   1.97
  M +1 -0      main.cpp   1.28

Another issue is what to do with applications that allow you to modify the file opened. In particular, what shall korganizer do if the user changes the events of the temp file? Just deleting the file after saving is not the best idea...

Cheers,
Reinhold

> Another issue is what to do with applications that allow you to modify the file opened. In particular, what shall korganizer do if the user changes the events of the temp file? Just deleting the file after saving is not the best idea...

kioexec takes care of "uploading after a tempfile was modified".
But here we're talking about --tempfile, which means "readonly as well"
(you don't want to edit a mail's attachment that way...).

Since --tempfile means "please delete after use", it certainly also means
"readonly". Does KOrganizer support a readonly mode?

Am Donnerstag, 11. November 2004 00:22 schrieb David Faure:
> > Another issue is what to do with applications that allow you to modify
> > the file opened. In particular, what shall korganizer do if the user
> > changes the events of the temp file? Just deleting the file after saving
> > is not the best idea...
>
> kioexec takes care of "uploading after a tempfile was modified".
> But here we're talking about --tempfile, which means "readonly as well"
> (you don't want to edit a mail's attachment that way...).
>
> Since --tempfile means "please delete after use", it certainly also means
> "readonly". Does KOrganizer support a readonly mode?

No, not really. KOrganizer doesn't have a full read-only mode.

KOrganizer supports read-only resources. However, loading one single file uses 
a CalendarLocal, which doesn't support read-only mode at all. So, the user 
will be able to edit the events, add incidences, etc. He can even save them 
(to the local file name), but all changes will be lost without warning.

Reinhold

> KOrganizer supports read-only resources. However, loading one single file uses 
> a CalendarLocal, which doesn't support read-only mode at all. So, the user 
> will be able to edit the events, add incidences, etc. He can even save them 
> (to the local file name), but all changes will be lost without warning.

Well, this is a request for a readonly mode in CalendarLocal then.
After all this might also be useful when reading a company-wide calendar
on a readonly shared folder (e.g. NFS). The same would happen there:
you can edit, but you can't save [ok in that case there would be a warning,
but still, a real readonly mode would be better].

*** Bug 95235 has been marked as a duplicate of this bug. ***

The original bug is fixed now, right?
I'll close this report now.
If you disagree, please reopen.
See also bug 130709.