Bug 91362 - wrong defaults for format/orientation in printsettingsdialogs of printerdrivers
Summary: wrong defaults for format/orientation in printsettingsdialogs of printerdrivers
Status: REOPENED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Printing and PDF export (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium minor
Assignee: Not Assigned
QA Contact:
URL:
Whiteboard: target:5.1.0 target:5.0.0.2 target:5...
Keywords:
Depends on: 63905
Blocks: Print-Dialog
  Show dependency treegraph
 
Reported: 2015-05-18 16:39 UTC by Christoph Lutz
Modified: 2016-10-18 12:16 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Lutz 2015-05-18 16:39:30 UTC
On Windows (32bit) and Linux (32bit) I noticed this bug that can be reproduced the following way:

a) create a new writer document
b) Format->Page and switch this page to Landscape-Format
c) File->Print / Properties... (opens the printer drivers settings dialog)

BUG: Default-Values for page-format don't reflect our landscape settings above. It shows "portrait". I would expect "landscape" here. If I do the workflow analogue in MS-Office, this dialog shows "landscape" instead of "portrait"

d) why is this behavíour annoying? because If I would like to change some other settings in the printer drivers dialog (e.g. duplex), I get the feeling that my results will come in with the wrong orientation.

Note: By default, only the page settings control in which size a page is printed. There is an printer option in the "options" tab called "use only paper size from printer preferences". Only if this option is switched on, the settings from the printer driver should be used.
Comment 1 Christoph Lutz 2015-05-18 17:16:47 UTC
Results of analysis:

1) THB stated that he couldn't reproduce this bug on linux 64bit - everything is as expected here (it is not yet sure, if this issue is related to to architecture 32/64 bit or just some other different settings - different printer drivers, ...)

2) The bug is from functional point of view related to tdf#61186 and commit 71ebe4404b6e7c78a7d2e352f6af88d57209680a

3) The bug is related to resetPaperToLastConfigured in http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/print3.cxx#818 . If this line is commented, then everything is fine, even with Win/Linux 32bit (but the use case described in 2. is broken, then).

4) We further noticed that in case of the bug, in line http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/print.cxx#1501 the variable bNeedToChange is true, even if if we don't want to trigger a "change". This is because of the comparison "maJobSetup.ImplGetConstData()->mnPaperWidth != aPageSize.Width() || maJobSetup.ImplGetConstData()->mnPaperHeight != aPageSize.Height()" which returns true. In this case, the compared width/height values are just slightly different (but logically "the same").

5) It seems we run into rounding errors. These rounding errors are caused in the mm/inch mapping PixelToLogic and show that the real cause seems to be much deeper inside the code. The PageSize Size-Structure used by mxPrinter can only hold integer (size in Pixels, stored as long) values which cannot be exactly converted to mm: 600 DPI / 2,54 inch * 21cm = 4960,62 (Pixels) --> This value is stored in an integer variable as 4961 and PixelToLogic (http://opengrok.libreoffice.org/xref/core/vcl/source/outdev/map.cxx#400) is not able to convert this value back without loss of information.
Comment 2 Buovjaga 2015-05-19 10:01:11 UTC
Reproduced, looking at the properties dialog of HP LaserJet 700 M775.

Win 7 Pro 64-bit Version: 5.0.0.0.alpha1+
Build ID: 158b50763962f66515062300e265839828463efa
TinderBox: Win-x86@39, Branch:master, Time: 2015-05-19_00:28:31
Locale: fi-FI (fi_FI)
Comment 3 Thorsten Behrens (CIB) 2015-05-19 12:14:39 UTC
Yep. Make sure to *not* use legal or letter both as the writer page format and for the printer, it seems you get lucky there with the rounding.

Can reproduce with a4 paper for sure.
Comment 4 Vasily Melenchuk (CIB) 2015-06-08 15:17:00 UTC
As far as I see from tdf#61186 and own experiments, it is not expected that printer settings dialog will show page orientation of document. It toggles *paper* rotation placed in *printer*, not a *page* rotation in *document*. Even corresponding tab in dialog is called "Paper", not "Page".

So in my mind behavior with A4 format is correct: orientation is not changed automatically, it can be adjusted by user. But Letter format works incorrectly: paper orientation is selected to be Landscape automatically, it can not be changed by user: once I select Portrait orientation, click Ok in Printer Properties Dialog and open it again, I expect to see Portrait orientation, and not the Landscape. 

Proposed fix is committed to gerrit: https://gerrit.libreoffice.org/#/c/16161/

After correcting of page sizes comparison in Printer::SetPaperSizeUser() everything became fine and it corresponds to logic written above.
Comment 5 Commit Notification 2015-06-25 09:47:56 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=07602e5a8b869be1c45158cf71d6015d17a5f797

tdf#91362: Don't override printer page autodetection

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2015-06-27 01:05:11 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1802e4c903a0597c9cb5c2ef752fe68486e197b1&h=libreoffice-5-0

tdf#91362: Don't override printer page autodetection

It will be available in 5.0.0.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 7 Buovjaga 2015-06-28 12:10:22 UTC
(In reply to Beluga from comment #2)
> Reproduced, looking at the properties dialog of HP LaserJet 700 M775.

Still repro.

Win 7 Pro 64-bit Version: 5.1.0.0.alpha1+
Build ID: 4fff776a08dd92121ab507a1ab506ac945abaedb
TinderBox: Win-x86@62-merge-TDF, Branch:MASTER, Time: 2015-06-28_00:25:54
Locale: fi-FI (fi_FI)
Comment 8 Commit Notification 2015-07-06 23:28:32 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0251e61640b94094918406b33ee7b05564409feb

tdf#91362: Make "printer was modified" status persistent

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2015-07-07 07:22:52 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bea3990141e598b60c358c03ec8f0e48ac15362b&h=libreoffice-5-0

tdf#91362: Make "printer was modified" status persistent

It will be available in 5.0.0.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Buovjaga 2015-07-07 13:23:29 UTC
Verified as fixed.

Win 7 Pro 64-bit Version: 5.1.0.0.alpha1+
Build ID: a625cd702700ae1773966a3133d27027d1c4d083
TinderBox: Win-x86@39, Branch:master, Time: 2015-07-07_08:23:06
Locale: fi-FI (fi_FI)
Comment 11 Commit Notification 2015-07-08 10:09:05 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6779b46796c93cbb4293a400f57c29e8ae85811b&h=libreoffice-5-0

Related tdf#91362: disable paper size & orientation selection

It will be available in 5.0.0.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2015-07-10 09:27:30 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2a1fe443a8343642292444be19cbd10700e7e01c

Related tdf#91362: disable paper size & orientation selection

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Cor Nouws 2015-07-10 23:17:26 UTC
(In reply to Commit Notification from comment #12)
> Katarina Behrens committed a patch related to this issue.
> It has been pushed to "master":

" It was really challenging to have this one toggle get all the way down through several layers of abstraction, though .. "

Hurray, lots of kudoos for hunting this all the way!
Comment 14 Christoph Lutz 2015-09-21 09:25:31 UTC
The use case with writer is fixed now. Unfortunately we figured out that the issue remains for impress. Here the steps to reproduce:

0) I have got a PDF printer installed that supports all common german page formats A4, A5, ... This printer is used for the test. This is done on linux.

1) create a new impress presentation

2) Format->Page and change page format to e.g. "A5" (I know, it is not typical to do such things)

3) File->Print --> now look at the preview and see, that A5 is not used now. I would expect to see the preview already in A5 format.

4) press button "Properties..." for the printersettings. Now I would expect the disabled comboboxes for size and orientation to show up "A5" and "landscape", but I get "A4" (my printeres preferred page size) and "landscape".


We also figured out, that the following patch (that is already included in libreoffice-5-0 and master) solves this issue for draw and impress:

commit 7b31e45ec7106d2cfbdbb7915d97667ba710f81c
Author: Eilidh McAdam <eilidh@lanedo.com>
Date:   Mon Jun 23 20:55:21 2014 +0100

    Make Draw use paper size when printing - fdo#63905
    
    Previously, Draw/Impress use the default size from the printer.
    Now Draw uses the paper size (specified in page formatting).
    Impress still uses the old method - not sure if this is correct
    but printing handouts etc probably complicate print/paper size.
    
    Change-Id: If90bae4ac59cd95dd50fcd8deb25fd900756193e
    Reviewed-on: https://gerrit.libreoffice.org/9866
    Reviewed-by: Matúš Kukan <matus.kukan@collabora.com>
    Tested-by: Matúš Kukan <matus.kukan@collabora.com>

and that this patch breaks the fix again for impress only:

commit f1f89f0202232635e7fbbd7ca47de51755b2bce0
Author: Caolán McNamara <caolanm@redhat.com>
Date:   Thu Sep 18 11:40:26 2014 +0100

    IsDraw doesn't mean the app/page is Draw
    
    it means a slide in impress.
    
    commit 7b31e45ec7106d2cfbdbb7915d97667ba710f81c
    Date:   Mon Jun 23 20:55:21 2014 +0100
        Make Draw use paper size when printing - fdo#63905
    
        Previously, Draw/Impress use the default size from the printer.
        Now Draw uses the paper size (specified in page formatting).
        Impress still uses the old method - not sure if this is correct
        but printing handouts etc probably complicate print/paper size.
    
    suggests the intent is for this to not affect Impress and to only
    affect Draw, so this does that
    
    Change-Id: I481a824ef244fd837992c893f6de0c051af0a26b

So my question is: What is this second patch supposed to do? which issue does it solve? Can we revert this one?
Comment 15 Thorsten Behrens (CIB) 2015-10-17 18:48:58 UTC
Adding involved developers.

@caolan: there was a question on f1f89f0202232635e7fbbd7ca47de51755b2bce0
Comment 16 Jan-Marek Glogowski 2015-12-02 08:48:51 UTC
Yesterday pinged caolan on IRC and he's fine with a revert of his patch. Will do when I have time to actually test this. But actually feel free to do the revert yourself but don't forget to test again the regression from bug 63905, which was somehow related to the last patch!