Fix usage of QPageLayout by calling the non default constructor
ClosedPublic

Authored by piggz on Jan 8 2017, 3:02 PM.

Details

Summary

Ensures internal unit is initialised.

Test Plan

Run kreportexample and ensure page width is as before. Change page size. Open existing reports in Kexi

Diff Detail

Repository
R14 KReport
Branch
fixup-qpagelayout
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz updated this revision to Diff 9858.Jan 8 2017, 3:02 PM
piggz retitled this revision from to Fix usage of QPageLayout by calling the non default constructor.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptJan 8 2017, 3:02 PM
staniek edited edge metadata.Jan 8 2017, 10:00 PM

Would you think it's possible to go for this idea? https://phabricator.kde.org/T4968#73569

piggz added a comment.Jan 8 2017, 10:09 PM

I can if you are certain. I thought because there was only 5 or so cases that it wasnt worth the effort and was better to stick closer to Qt ?

In D4021#75291, @piggz wrote:

I can if you are certain. I thought because there was only 5 or so cases that it wasnt worth the effort and was better to stick closer to Qt ?

I am rather certain -- it's centralized change. We will able for example to add an #ifdef for Qt that fixes the issue, once there's one. I propose to add a KReportPrivate::PageLayout class in KReportUtils_p.h.

piggz updated this revision to Diff 10335.Jan 18 2017, 8:39 PM
piggz edited edge metadata.
  • Fix by sub-classing QPageLayout
staniek requested changes to this revision.Jan 18 2017, 9:23 PM
staniek edited edge metadata.
staniek added inline comments.
src/common/KReportDocument.cpp
41

By the way: page -> pageLayout

src/common/KReportUtils_p.h
172

+add

//! This class is wrapper that fixes a critical QTBUG-47551 bug in default constructor of QPageLayout
//! Default constructor of QPageLayout does not initialize units.
//! https://bugreports.qt.io/browse/QTBUG-47551
//! @todo remove this class and go back to QPageLayout when the faulty QPageLayout implementations are no longer on the wild. That's probably for Qt 6.
174

For completeness PageLayout(const QPageLayout &pageLayout) could be nice (nonexplicit).

This revision now requires changes to proceed.Jan 18 2017, 9:23 PM
piggz updated this revision to Diff 10337.Jan 18 2017, 10:08 PM
piggz edited edge metadata.
  • Add comments and ctor
staniek accepted this revision.Jan 18 2017, 11:53 PM
staniek edited edge metadata.

One change left and please push without further review.

src/common/KReportDocument.cpp
41

not done?

This revision is now accepted and ready to land.Jan 18 2017, 11:53 PM