Bug 77574

Created attachment 5217 [details]
this patch sets the correct URL for the document after loading the temporary converted file

The patch applies to $KOFFICE_SRC/koshell/koshell_shell.cc

At the end of the open method a check for a temporary file is made which
indicates a conversion from a non-native format. In this case we set the URL
and file (the same??) for the document to the original file and correct the
mimetype to the non-native type.

I also tried to make KOffice show a warning when saving because it's in
non-native type but these two calls don't seem to have any effekt - someone
with more knowledge might have a look at it. So CURRENTLY THE USER IS NOT
WARNED when (s)he again saves the document in the non-native format!! Apart
from that the patch is tested and solves the observed bug(s) (that is, the
settings correction of the imported document).

Regards,
Raphael

The code in KoMainWindow changed indeed - did you compare with the code there?
The duplication is unfortunate (but hard to avoid), but basically both should behave the same... (I just wonder if the answer to the FIXME:... is there).
Hmm, adding Clarence do the CC, who is responsible for the koMainWindow changes.

On Tue, 16 Mar 2004 05:30 am, David Faure wrote:
> ------- Additional Comments From faure kde org  2004-03-15 19:30 -------
> The code in KoMainWindow changed indeed - did you compare with the code
> there? The duplication is unfortunate (but hard to avoid), but basically
> both should behave the same... (I just wonder if the answer to the
> FIXME:... is there). Hmm, adding Clarence do the CC, who is responsible for
> the koMainWindow changes.
Sorry about the bug.  I'll try to reproduce and fix on the weekend when I'm 
not completely busy.

Clarence

Ok, I've had a look into it:

This bug has been uncovered due to another bug:

The line "m_documentEntry = KoDocumentEntry::queryByMimeType( mimeType->name().latin1() )" in KoShellWindow::openDocumentInternal() complains:

koffice (lib kofficecore): WARNING: Got no results with [X-KDE-NativeMimeType] == 'application/vnd.sun.xml.writer'
koffice (lib kofficecore): ERROR: Found no KOffice part able to handle application/vnd.sun.xml.writer!
koffice (lib kofficecore): ERROR: Check your installation (does the desktop file have X-KDE-NativeMimeType and KOfficePart, did you install KOffice in a different prefix than KDE, without adding the prefix to /etc/kderc ?)

which forces m_documentEntry to be empty.  This should be fixed first?  Something wrong with the oowriter mimetype, I guess.

The comment "// non-native" in the code is misleading because e.g. if you open an mswrite file, it gives a documentEntry which is not empty.


Your patch looks right but you need to change KoDocument's mimeType, not just outputMimeType (press File/Save As after opening to see what I mean).

Hmm, looks like remote documents don't work at all (not caused by your patch though).

The setConfirmNonNativeSave() calls are correct and seem to work for me.  Are you sure you didn't click that DontAskAgain?

On Saturday 20 March 2004 10:16, Clarence Dang wrote:
> Ok, I've had a look into it:

thanks!!

>
> This bug has been uncovered due to another bug:
>
> The line "m_documentEntry = KoDocumentEntry::queryByMimeType(
> mimeType->name().latin1() )" in KoShellWindow::openDocumentInternal()
> complains:
>
> koffice (lib kofficecore): WARNING: Got no results with
> [X-KDE-NativeMimeType] == 'application/vnd.sun.xml.writer' koffice (lib
> kofficecore): ERROR: Found no KOffice part able to handle
> application/vnd.sun.xml.writer! koffice (lib kofficecore): ERROR: Check
> your installation (does the desktop file have X-KDE-NativeMimeType and
> KOfficePart, did you install KOffice in a different prefix than KDE,
> without adding the prefix to /etc/kderc ?)
>
> which forces m_documentEntry to be empty.  This should be fixed first? 
> Something wrong with the oowriter mimetype, I guess.

I'm not familiar with this, but it surely needs to be fixed one way or the 
other.

>
> The comment "// non-native" in the code is misleading because e.g. if you
> open an mswrite file, it gives a documentEntry which is not empty.

what do you propose?

>
>
> Your patch looks right but you need to change KoDocument's mimeType, not
> just outputMimeType (press File/Save As after opening to see what I mean).

I had a quick look at the classes... how "can" the mimetype of an open 
document be changed? For me it works - the saving works correctly into the 
original mime type of the file.

>
> Hmm, looks like remote documents don't work at all (not caused by your
> patch though).
>
> The setConfirmNonNativeSave() calls are correct and seem to work for me. 
> Are you sure you didn't click that DontAskAgain?

You're right, just manually checked the koshellrc file. Updated and tested 
again. It works :-)

concluding: still open stuff (todos):
* fix mime type settings for oowriter (external bug)
* change document's mime type (should be fixed in this bug hunting)
* fix remote files (external bug)
* provide a better comment (see above, should also be fixed in this bug 
hunting)

PS: thanks for having a look, apart from providing some answers I can't do 
much more (mostly a "not enough time" problem), the bug - IMHO - is 
(partially) fixed and has no regressions (AFAIK), so it could be used. If you 
know for a better comment ("non-native") then please go ahead, I'm no expert 
and would appreciate every help.

Regards,
Raphael

On Sat, 20 Mar 2004 11:14 pm, Raphael Langerhorst wrote:
> > This bug has been uncovered due to another bug:
> >
> > The line "m_documentEntry = KoDocumentEntry::queryByMimeType(
> > mimeType->name().latin1() )" in KoShellWindow::openDocumentInternal()
> > complains:
> >
> > koffice (lib kofficecore): WARNING: Got no results with
> > [X-KDE-NativeMimeType] == 'application/vnd.sun.xml.writer' koffice (lib
> > kofficecore): ERROR: Found no KOffice part able to handle
> > application/vnd.sun.xml.writer! koffice (lib kofficecore): ERROR: Check
> > your installation (does the desktop file have X-KDE-NativeMimeType and
> > KOfficePart, did you install KOffice in a different prefix than KDE,
> > without adding the prefix to /etc/kderc ?)
> >
> > which forces m_documentEntry to be empty.  This should be fixed first?
> > Something wrong with the oowriter mimetype, I guess.
>
> I'm not familiar with this, but it surely needs to be fixed one way or the
> other.
>
> > The comment "// non-native" in the code is misleading because e.g. if you
> > open an mswrite file, it gives a documentEntry which is not empty.
>
> what do you propose?
>

   ^
   |
   |
dfaure :)

> > Your patch looks right but you need to change KoDocument's mimeType, not
> > just outputMimeType (press File/Save As after opening to see what I
> > mean).
>
> I had a quick look at the classes... how "can" the mimetype of an open
> document be changed? 
By adding a setMimeType() method to KoDocument to go with the setFile() hack 
:)  Actually, the setFile() hack won't be needed once KOffice requires KDE 
3.3 but that's another matter.

> For me it works - the saving works correctly into the
> original mime type of the file.
Look at the default output format when you click File / Save As.  File / Save 
is ok though.

> * fix remote files (external bug)
I'll look at this one on the weekend unless someone else beats me to it :)

Clarence

On Monday 22 March 2004 10:53, Clarence Dang wrote:
>  [...]

ok, attached are my final versions for the problem. I modified KoDocument to 
have a setMimeType member to allow setting the mime type during loading in 
koshell. (Now the correct mime type is selected by default when choosing 
"save as")

All works, no (experienced) regressions, problem solved, please review and 
apply.

Also note: since I changed the KoDocument class this breaks binary 
compatibility(!!)

Regards,
Raphael


Created an attachment (id=5393)
koshell_correct_URL_version2_kodocument.patch

Created an attachment (id=5394)
koshell_correct_URL_version2_koshell.patch

On Thursday 25 March 2004 20:28, Raphael Langerhorst wrote:
> All works, no (experienced) regressions, problem solved, please review and 
> apply.

Great, thanks much.

> Also note: since I changed the KoDocument class this breaks binary 
> compatibility(!!)
Not at all, a new non-virtual method keeps BC. The change is fine.

I'll apply to head and branch.

CVS commit by faure: 

Applying patch by Raphael Langerhorst to fix
[Bug 77574] koshell sets the wrong URL (the temporary converted file!!) for imported documents

CCMAIL: 77574-done@bugs.kde.org


  M +24 -0     koshell/koshell_shell.cc   1.72.2.1
  M +3 -0      lib/CHANGES   1.17.2.15
  M +5 -0      lib/kofficecore/koDocument.cc   1.287.2.7
  M +7 -0      lib/kofficecore/koDocument.h   1.147.2.2


--- koffice/koshell/koshell_shell.cc  #1.72:1.72.2.1
@@ -190,4 +190,28 @@ bool KoShellWindow::openDocumentInternal
 
   if ( tmpFile ) {
+    //if the laoded file has been a temporary file
+    //we need to correct a few document settings
+    //see description of bug #77574 for additional information
+  
+    //correct (output) mime type: we need to set it to the non-native format
+    //to make sure the user knows about saving to a non-native mime type
+    //setConfirmNonNativeSave is set to true below
+    newdoc->setMimeType( mimeType->name().latin1() );
+    newdoc->setOutputMimeType( mimeType->name().latin1() );
+    
+    //the next time the user saves the document he should be warned
+    //because of mime type settings done above;
+    newdoc->setConfirmNonNativeSave(true,true); //exporting,warn_on
+    newdoc->setConfirmNonNativeSave(false,true); //save/save as,warn_on
+    
+    //correct document file (should point to URL)
+    newdoc->setFile( url.path() );
+    
+    //correct document URL
+    newdoc->setURL( url );
+    
+    //update caption to represent the correct URL in the window titlebar
+    updateCaption();
+  
     tmpFile->unlink();
     delete tmpFile;

--- koffice/lib/CHANGES  #1.17.2.14:1.17.2.15
@@ -36,4 +36,7 @@
 - Fixed infinite loop on invalid WMF file
 
+KoShell:
+- Fixed problems (wrong URL etc.) after importing non-native documents (#77574)
+
 Changes after KOffice-1.3-rc1
 =============================

--- koffice/lib/kofficecore/koDocument.cc  #1.287.2.6:1.287.2.7
@@ -462,4 +462,9 @@ QCString KoDocument::mimeType() const
 }
 
+void KoDocument::setMimeType( const QCString & mimeType )
+{
+    d->mimeType = mimeType;
+}
+
 void KoDocument::setOutputMimeType( const QCString & mimeType, int specialOutputFlag )
 {

--- koffice/lib/kofficecore/koDocument.h  #1.147.2.1:1.147.2.2
@@ -199,4 +199,11 @@ public:
 
     /**
+     * Sets the mime type for the document.
+     * When choosing "save as" this is also the mime type
+     * selected by default.
+     */
+    void setMimeType( const QCString & mimeType );
+
+    /**
      * Set the format in which the document should be saved.
      * This is called on loading, and in "save as", so you shouldn't