Bug 412366 - Print to File (PDF) broken on KDE4 in 4.8.4
Summary: Print to File (PDF) broken on KDE4 in 4.8.4
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: reports (show other bugs)
Version: 4.8.4
Platform: Slackware Linux
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-26 15:04 UTC by Erich
Modified: 2019-10-24 08:13 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.8.5
ralf.habacker: Version_5-


Attachments
Restore "Print to File (PDF)" functionality for KDE4 (464 bytes, patch)
2019-09-26 15:04 UTC, Erich
Details
Patch for kdelibs to support requested print operation (2.43 KB, patch)
2019-09-26 17:51 UTC, Ralf Habacker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erich 2019-09-26 15:04:56 UTC
Created attachment 122879 [details]
Restore "Print to File (PDF)" functionality for KDE4

SUMMARY
Commit 15ac7d472af41fc503dc5209643cfc8b392b0089, introduced in KMyMoney 4.8.4, breaks Print to File (PDF) functionality on KDE4 in Slackware64 14.2.  Reverting that commit on branch 4.8 HEAD restores Print to File (PDF) functionality.

STEPS TO REPRODUCE
1. Choose Reports, 1. Income and Expenses -> Income and Expenses This Month (Default Report)
2. Then choose Print, Print to File (PDF).

OBSERVED RESULT
KMyMoney goes through all the motions - including asking for confirmation to overwrite an existing file - but the PDF does not get created.  Versions 4.8.3 and earlier create the PDF as expected.


EXPECTED RESULT
PDF file gets created by KMyMoney.

SOFTWARE/OS VERSIONS
Linux: Slackware64 14.2
KDE Platform Version: 4.14.38
Qt Version: 4.8.7

ADDITIONAL INFORMATION

I also developed a small patch which restores Print to File (PDF) functionality for me, but the print dialog appears twice (I did expect that to happen when I tried out the patch).  So it is not a proper patch.

What I believe is the cause of the problem:
The net result of patches:
5f4f90e83565647f3579660e4c5d9ac447547a02 Fix 'Printer settings are not saved'
15ac7d472af41fc503dc5209643cfc8b392b0089 Add print support for report charts
Changes the "print" command from
m_part->view()->print()
to
q->m_part->view()->print(true);
when KDE_IS_VERSION(4, 14, 65) is false (which is the case for me).

From what I can tell from reading the docs (I am not a KDE developer), this changes the "print" command from "print by asking the user how to print" to "print using the default settings".  Unfortunately, those "default settings" are not what KMyMoney asked about in the code immediately preceding the "print" command, they must be the system-wide defaults.  My minimally-invasive patch simply changes the print command back to what it used to be, with the net effect that the print dialog appears twice when printing a report.

I did not test printing charts, which is what 15ac7d472af41fc503dc5209643cfc8b392b0089 addressed in the first place.

Another Slackware user was able to reproduce the issue.  See thread at:
https://lists.slackbuilds.org/pipermail/slackbuilds-users/2019-September/023367.html
Comment 1 Ralf Habacker 2019-09-26 17:51:21 UTC
Created attachment 122884 [details]
Patch for kdelibs to support requested print operation

On Windows builds the appended patch add related support to kdelibs. This need to be applied to Slackware's kdelibs4 package.
Comment 2 Erich 2019-09-26 18:07:55 UTC
Hi Ralf,

I will try your patch.

Are there any concerns with updating KDE_VERSION_RELEASE to 65?  The patch file increases it from 60 to 65, whereas Slackware has it at 38.  I assume I'll be missing additional functionality that was added along the way?

Where can I find these additional patches to kdelibs?  I was under the impression that 4.14.38 was the last release of kdelibs.

Erich
Comment 3 Ralf Habacker 2019-09-26 18:28:09 UTC
(In reply to Erich from comment #0)
> What I believe is the cause of the problem:
> The net result of patches:
> 5f4f90e83565647f3579660e4c5d9ac447547a02 Fix 'Printer settings are not saved'
> 15ac7d472af41fc503dc5209643cfc8b392b0089 Add print support for report charts
> Changes the "print" command from
> m_part->view()->print()
> to
> q->m_part->view()->print(true);
> when KDE_IS_VERSION(4, 14, 65) is false (which is the case for me).
    /**
     * Prints the HTML document.
     * @param quick if true, fully automated printing, without print dialog
     */
    void print( bool quick = false );

> From what I can tell from reading the docs (I am not a KDE developer), this
> changes the "print" command from "print by asking the user how to print" to
> "print using the default settings".  Unfortunately, those "default settings"
> are not what KMyMoney asked about in the code immediately preceding the
> "print" command, they must be the system-wide defaults.  
Thanks for reporting this hidden issue.

> My minimally-invasive patch simply changes the print command back to what it
> used to be, with the net effect that the print dialog appears twice when
> printing a report.
This could be fixed with the patch from attachment 122884 [details].

> I also developed a small patch which restores Print to File (PDF)
> functionality for me, but the print dialog appears twice (I did expect that
> to happen when I tried out the patch).  So it is not a proper patch.
which is required as fallback to avoid this issue, if the mentioned patch is not applied to kdelibs4.
Comment 4 Ralf Habacker 2019-09-26 18:36:23 UTC
(In reply to Erich from comment #2)
> Hi Ralf,
> 
> I will try your patch.
> 
> Are there any concerns with updating KDE_VERSION_RELEASE to 65?  The patch
> file increases it from 60 to 65, whereas Slackware has it at 38.  I assume
> I'll be missing additional functionality that was added along the way?
Mostly fixes

> Where can I find these additional patches to kdelibs? 
At https://build.opensuse.org/package/show/windows:mingw:win32/mingw32-kdelibs4 there is a tarball for 4.14.60 and several patches I collected in the past. 

To be find kdelibs4 with method print(QPrinter* printer, bool quick) I raised the kdelibs patch level to 65.
Comment 5 Ralf Habacker 2019-09-26 18:47:40 UTC
(In reply to Ralf Habacker from comment #4)

> At https://build.opensuse.org/package/show/windows:mingw:win32/mingw32-kdelibs4
> there is a tarball for 4.14.60 and several patches I collected in the past.
Most patches are intended for cross compiling windows packages.
Comment 6 Ralf Habacker 2019-09-26 18:53:53 UTC
Git commit beca6d92002eabc52ebd1da52d6edf4c06c9c22c by Ralf Habacker.
Committed on 26/09/2019 at 18:52.
Pushed by habacker into branch '4.8'.

Fix 'Print to File (PDF) broken on KDE4 in 4.8.4 on Slackware'

Using the KHtmlView quick print support does not work if a user does
not select the default system printer in kmymoney.
FIXED-IN:4.8.5

M  +1    -1    kmymoney/views/kreportsview.cpp

https://commits.kde.org/kmymoney/beca6d92002eabc52ebd1da52d6edf4c06c9c22c
Comment 7 Erich 2019-09-29 16:13:20 UTC
I can confirm that the following all work:

Attachment 122884 [details] (https://bugs.kde.org/attachment.cgi?id=122884) to kdelibs adds the required functionality to kdelibs so that KMyMoney 4.8.4 print works correctly.  (Of course KMyMoney must be recompiled after upgrading kdelibs so that the new behavior to print() is enabled.)

OR

Commit beca6d920 (https://cgit.kde.org/kmymoney.git/commit/?id=beca6d92002eabc52ebd1da52d6edf4c06c9c22c) restores print functionality on stock Slackware64 14.2.  The print dialog appears twice, but this is acceptable for me.
Comment 8 Wolfgang Bauer 2019-09-30 06:20:04 UTC
This additional patch would avoid showing the print dialog twice with an unpatched kdelibs:
diff --git a/kmymoney/views/kreportsview.cpp b/kmymoney/views/kreportsview.cpp
index 5adcdb2..4afc024 100644
--- a/kmymoney/views/kreportsview.cpp
+++ b/kmymoney/views/kreportsview.cpp
@@ -152,9 +152,11 @@
 
 void KReportsView::KReportTab::print()
 {
+#if KDE_IS_VERSION(4, 14, 65)
   QPrintDialog dlg(kmymoney->printer(), this);
   if (!dlg.exec())
     return;
+#endif
 
   d->slotPaintRequested(kmymoney->printer());
 }
Comment 9 Ralf Habacker 2019-10-01 17:11:26 UTC
Git commit 195f84d09633ecd8939468ce455c793048ca4ab5 by Ralf Habacker, on behalf of Wolfgang Bauer.
Committed on 01/10/2019 at 17:11.
Pushed by habacker into branch '4.8'.

Avoid showing the print dialog twice with an unpatched kdelibs

M  +2    -0    kmymoney/views/kreportsview.cpp

https://commits.kde.org/kmymoney/195f84d09633ecd8939468ce455c793048ca4ab5
Comment 10 Ralf Habacker 2019-10-01 17:20:42 UTC
(In reply to Wolfgang Bauer from comment #8)
Thanks for providing this patch

Is there any chance that someone can apply and check attachment 122967 [details] if it works with unpatched kdelibs ? This patch provides print preview support and I would  add it to the next release.
Comment 11 Erich 2019-10-01 19:09:16 UTC
(In reply to Ralf Habacker from comment #10)
> Is there any chance that someone can apply and check attachment 122967 [details]
> [details] if it works with unpatched kdelibs ? This patch provides print
> preview support and I would  add it to the next release.

Yes I will test it later today.
Comment 12 Erich 2019-10-01 22:23:02 UTC
(In reply to Ralf Habacker from comment #10)
> 
> Is there any chance that someone can apply and check attachment 122967 [details]
> [details] if it works with unpatched kdelibs ? This patch provides print
> preview support and I would  add it to the next release.

I tested attachment 122967 [details] on top of branch 4.8 (195f84d09 Avoid showing the print dialog twice with an unpatched kdelibs).

kdelibs is version 4.14.38.

Print preview works correctly for report charts.  Printing from the print preview dialog works correctly.

However, print preview for anything else (report tables and home screen tested) brings up the print dialog and KMyMoney goes through the print process instead.  If printing to a file, the PDF is created as if "Print" were chosen instead.  THEN, the print preview windows shows up, showing the last "successful" print preview.  This is either blank, or if a print preview was previously generated for a report chart, that report chart is shown in the print preview dialog.

My opinion as a user:  Print preview is new functionality, so previously it wasn't even an option.  KDE4 is extremely old at this point.  If most of your users of KMyMoney 4.8 are on kdelibs >= 4.14.65 and print preview works correctly there, you should just keep your patch as is.  When I asked about my original bug report on the slackbuilds mailing list, only 1 person responded, and he doesn't even use the Print to File feature.  Eventually Slackware 15 will be released (nobody knows when...) and I can only assume it will have Qt5/KDE5, at which point I would move to KMyMoney 5.  So I don't care if print preview is broken for everything except report charts.  I wouldn't want you to waste time trying to make it work correctly just for me if it already works for all other users.
Comment 13 Ralf Habacker 2019-10-07 18:03:37 UTC
(In reply to Erich from comment #12)
Thanks for reporting.

> However, 
Then it looks that disabling print preview for unpatched kdelibs is the secure way. 

> Qt5/KDE5, at which point I would move to KMyMoney 5. 
KHtmlView/KF5 provides the QPainter parameter. This means that there will be no issue, if print preview would be added to KF5 port.
Comment 14 Ralf Habacker 2019-10-11 10:39:16 UTC
Git commit 04af112832af9307c6a2c82bb83e8bf993a19433 by Ralf Habacker.
Committed on 11/10/2019 at 10:38.
Pushed by habacker into branch '4.8'.

Disable the print preview with an unpatched kdelibs to avoid problems with the print settings

See https://bugs.kde.org/show_bug.cgi?id=412366#c12 for details
Related: bug 406338

M  +4    -0    kmymoney/kmymoney.cpp

https://commits.kde.org/kmymoney/04af112832af9307c6a2c82bb83e8bf993a19433
Comment 15 Ralf Habacker 2019-10-14 07:47:25 UTC
(In reply to Ralf Habacker from comment #13)
> > Qt5/KDE5, at which point I would move to KMyMoney 5. 
> KHtmlView/KF5 provides the QPainter parameter. This means that there will be
> no issue, if print preview would be added to KF5 port.

I need to correct this statement: KHTMLView/KF5 also do not has a print method with QPrinter parameter (see https://cgit.kde.org/khtml.git/tree/src/khtmlview.h#n200). I filed a bug report for this (see https://bugs.kde.org/show_bug.cgi?id=405011)
Comment 16 Wolfgang Bauer 2019-10-17 20:04:28 UTC
(In reply to Ralf Habacker from comment #15)
> (In reply to Ralf Habacker from comment #13)
> > > Qt5/KDE5, at which point I would move to KMyMoney 5. 
> > KHtmlView/KF5 provides the QPainter parameter. This means that there will be
> > no issue, if print preview would be added to KF5 port.
> 
> I need to correct this statement: KHTMLView/KF5 also do not has a print
> method with QPrinter parameter (see
> https://cgit.kde.org/khtml.git/tree/src/khtmlview.h#n200). I filed a bug
> report for this (see https://bugs.kde.org/show_bug.cgi?id=405011)

But kmymoney 5.x doesn't use KHTML anymore. It can either be built with QtWebKit or QtWebEngine.
Or am I misunderstanding something here?
Comment 17 Ralf Habacker 2019-10-24 08:13:15 UTC
(In reply to Wolfgang Bauer from comment #16)
> But kmymoney 5.x doesn't use KHTML anymore. It can either be built with
> QtWebKit or QtWebEngine.
I can confirm that. Unfortunately with the transition to QtWebKit/QWebEngine the printout of headers and footers gets lost, see bug 413352.