- R8:18bfd13a8020: Fixed calligra crashing when opening remote document
Example working print: calligrawords --print "https://www.oasis-open.org/committees/download.php/31049/UOML%20Sample.odt"
Calligra was crashing because the method "slotFilePrint" called "rootView()" that tried to return the first elements of "d->rootViews". The problem is that such list was empitya, as SetRootDocument was never called: that method is called in openDocument but not in openDocumentInternal. I therefore added that function and it now works. But, when slotLoadCompleted is called (that is, after slotFilePrint), there's a check to open a new window if there's already a non-blank document. Since the root document was already set, it opened a new window with the same document. I therefore added the conditions of the two documents being different.
I hope I've been clear, this is one of my first commits here :-/
At this point my only worry is that the setRootDocument in openDocumentInternal makes the check for opening a new window in slotLoadCompleted useless. Moving the whole if statement from slotLoadCompleted to openDocumentInternal also works, and could avoid to replace the root document instead of opening a new window. It makes more sense, now that I think of it.
Yes that description helped a lot, and you are doing great. Keep up the good work :)
One concern though - how often and when is openDoumentInternal called. I'm a bit afraid that we just add some functionality to a place that might be used for something else
Please check with loading a normal document,. The same concerns now applies to slotLoadCompleted - especially since you have now removed functionality from it.
Also please check in all of our applications as some might reimplement virtual methods
I tried differents files (locale and remote, opening and printing), and everything works okay. Other applications work normally, but calligrasheets still crashes when trying to print a remote (odt) file - but I can see the file is downloaded correctly, so it looks like a different problem (calligrasheets manages printing?).
About removing features from slotLoadCompleted, it is important to notice that it is only called by openDocumentInternal itself, so that should not be a problem.
I think the problem is that slotFilePrint() is called (in KoApplication::start()) before the document is actually loaded.
Probably we need to start printing in a slot connected to KoMainWindow::loadCompleted() instead.
And to be safe, we also need to handle loadCanceled().
And the same for exportToPdf, I think.
I'm not sure on how to do that, do you have any tip?
openDocumentInternal is called by openDocument, which is called in KoApplication just before checking the arguments. When it sees the print argument, it calls slotFilePrint. openDocumentInternal downloads the document in async. Should it be made not async, or is there another way to make it wait until the other process ends? Also, "slotLoadCanceled" is already implemented, or were you talking about something else?
Well niccolove, you have really put your hand into a bee-hive :)
The answer from anthony can be used as a test to see if the the princip works, but does not address all situations.
The current implementation needs quite a lot of change to work. These are the conditions as I see them:
- openDocument() initiates loading and returns true if it finds the file, just return false if the file do not exist.
- The actual loading is asynchronous if the file is remote.
- It is possible to specify multiple files to load. (Each file will be shown in it's own main window.)
- For writing to pdf, a file name can be specified on the command line.
These are the steps I think is needed:
- Create slot(s) in KoApplication to handle the different resullts; print, pdf and cancelled.
- The signal connections must be done before openDocument() is called. (So that local files also works.)
- Add KoDocument as an arg to the KoMainWindow::loadCompleted() signal, so KoApllication knows which dcoument has been loaded.
- For pdf, find a way to handle filename.
What have I Done:
- Added slots in KoApplication to manage pdf and print. Cancel is already KoMainWindow::slotLoadCanceled with an error popup, what should a KoApplication::slotLoadCanceled do?
- Added connection for pdf print and moved it before document opening (local files works)
- I added the KoMainView to the loadCompleted signal, as a new one might be created in KoMainWindow::slotLoadCompleted, and the slots needs the new one to call slotFilePrint or exportToPdf.
- Filename is passed to slot using a lambda, like specified on the slots and signals qt documentation
Things seem working. Should I do anything else?
regarding the lambda connection - nice to see that you use the version with a "still alive" argument - but I fear this requires a qt version higher than what we currently support - could you please check
Regarding the completed slot, the doc says:
--print Only print and exit
--export-pdf Only export to PDF and exit
To exit, you need to know when all jobs have finished, so if some failed you need to track that.
Also, the connection to job in slotExportToPdf() is wrong (not your fault) as it will just quit while there may be other jobs running.
I have some doubts:
- Anything related to "and exiting" does not seem to work. mainWindow->slotFileQuit() does not seem working.
- --benchmark-loading-show-window should also exit using mainWindow->slotFileQuit(), but it doesn't.
- Adding mainWindow->slotFileQuit() after a print job does nothing.
- Calligra does not seem to exit after pdf job, even though job end is connected to mainWindow->slotFileQuit(). (removing the connect does not seem to change anything)
- The exit after exporting to pdf job seems right, since you should not be able to export multiple files to a single pdf anyway, and you can't print AND export to pdf in the same command.
- The print command will open a mainWindow for every printing job, so running mainWindow->slotFileQuit() after mainWindow->slotFilePrint() would bring to having no windows open at the end of all jobs.
Yeah, well there are some half done designs in here :(
The benchmark just does not work with remote files. Don't know if it should or if it should be blocked. Let's leave it for now.
If you do not specify a pdf filename, it will use the same name as in the loaded file just changing the extension to .pdf, so it is possible to export multiple files.
I have sort of assumed that print and exportToPdf commands was meant to be hands free, to be able to print a number of files wo a lot of mouse work.
Popping up dialogs (print dialog or error dialogs) defies that goal, so imho these things should be handled differently.
I recognize that this fix has balooned and we are getting into difficult areas, so I propose:
Let's do without exit atm, just remove the job connection in slotExportToPdf() and add a TODO to implement exit later.
When export to pdf, the mainwindow is not shown and since we don't exit, the app will hang.
You need to invert the check in KoApplication line 460 so the window is shown.
Also please fix the trailing spaces.
And then: ok to land!
Thanks, I'm checking it out now...
Just a couple of things:
Keywords needs to be on a separate line to have effect, see https://community.kde.org/Infrastructure/Git/Hooks
Also FIXED-IN is nice to add.
I've updated and closed the bug, so no problem.