fix the orientation combobox in the print dialog

Changing the orientation value to anything else than 'Automatic'
didn't do anything by default. This was because by default
neither isPaperSizeFromUser() nor getPapersizeFromSetup()
were set, so PrintDialog::setPaperOrientation() did nothing.
It looks to me like 8cbdc6a068ad88fc43a98bd0f88 that introduced it
was rather broken (not just this bug, but also e.g. the ugly
modifying of the paper sizes by non-const reference from a const
function). In fact this whole stuff still looks broken to me, why
does it change paper size instead of just setting the orientation?
It seems like the orientation gets reset, or maybe the setting
was just a band-aid. I don't know how to fix that all though.

Change-Id: If5fdf4c47e06f2b0797d27126d21b3451b8334cf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129241
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

3 files changed, 38 insertions, 53 deletions

diff --git a/vcl/inc/printdlg.hxx b/vcl/inc/printdlg.hxx
index db1e6a73bcfc..6de7cbfb68fb 100644
--- a/vcl/inc/printdlg.hxx
+++ b/vcl/inc/printdlg.hxx
@@ -231,10 +231,9 @@ namespace vcl
         void setupPaperSidesBox();
         void storeToSettings();
         void readFromSettings();
-        void setPaperOrientation( Orientation eOrientation );
+        void setPaperOrientation( Orientation eOrientation, bool fromUser );
         void updateOrientationBox( bool bAutomatic = true );
         bool hasOrientationChanged() const;
-        void checkPaperSize( Size& rPaperSize );
         void setPreviewText();
         void updatePrinterText();
         void checkControlDependencies();
diff --git a/vcl/source/gdi/print3.cxx b/vcl/source/gdi/print3.cxx
index 8b00e2d9f85b..f86b8b21ae3d 100644
--- a/vcl/source/gdi/print3.cxx
+++ b/vcl/source/gdi/print3.cxx
@@ -155,6 +155,7 @@ public:
     bool                                                        mbReversePageOrder;
     bool                                                        mbPapersizeFromSetup;
     bool                                                        mbPapersizeFromUser;
+    bool                                                        mbOrientationFromUser;
     bool                                                        mbPrinterModified;
     css::view::PrintableState                                   meJobState;
 
@@ -168,6 +169,8 @@ public:
     Size                                                        maDefaultPageSize;
     // set by user through print dialog
     Size                                                        maUserPageSize;
+    // set by user through print dialog
+    Orientation                                                 meUserOrientation;
     // set by user through printer properties subdialog of printer settings dialog
     sal_Int32                                                   mnDefaultPaperBin;
     // Set by user through printer properties subdialog of print dialog.
@@ -194,6 +197,7 @@ public:
         mbReversePageOrder( false ),
         mbPapersizeFromSetup( false ),
         mbPapersizeFromUser( false ),
+        mbOrientationFromUser( false ),
         mbPrinterModified( false ),
         meJobState( css::view::PrintableState_JOB_STARTED ),
         mnDefaultPaperBin( -1 ),
@@ -209,15 +213,27 @@ public:
         }
     }
 
-    const Size& getRealPaperSize( const Size& i_rPageSize, bool bNoNUP ) const
+    Size getRealPaperSize( const Size& i_rPageSize, bool bNoNUP ) const
     {
+        Size size;
         if ( mbPapersizeFromUser )
-            return maUserPageSize;
-        if( mbPapersizeFromSetup )
-            return maDefaultPageSize;
-        if( maMultiPage.nRows * maMultiPage.nColumns > 1 && ! bNoNUP )
-            return maMultiPage.aPaperSize;
-        return i_rPageSize;
+            size = maUserPageSize;
+        else if( mbPapersizeFromSetup )
+            size =  maDefaultPageSize;
+        else if( maMultiPage.nRows * maMultiPage.nColumns > 1 && ! bNoNUP )
+            size = maMultiPage.aPaperSize;
+        else
+            size = i_rPageSize;
+        if(mbOrientationFromUser)
+        {
+            if ( (meUserOrientation == Orientation::Portrait && size.Width() > size.Height()) ||
+                 (meUserOrientation == Orientation::Landscape && size.Width() < size.Height()) )
+            {
+                // coverity[swapped-arguments : FALSE] - this is in the correct order
+                size = Size( size.Height(), size.Width() );
+            }
+        }
+        return size;
     }
     PrinterController::PageSize modifyJobSetup( const css::uno::Sequence< css::beans::PropertyValue >& i_rProps );
     void resetPaperToLastConfigured();
@@ -821,6 +837,7 @@ void PrinterController::setPrinter( const VclPtr<Printer>& i_rPrinter )
     }
 
     mpImplData->mbPapersizeFromUser = false;
+    mpImplData->mbOrientationFromUser = false;
     mpImplData->mxPrinter->Pop();
     mpImplData->mnFixedPaperBin = -1;
 }
@@ -1404,7 +1421,10 @@ void PrinterController::setPapersizeFromSetup( bool i_bPapersizeFromSetup )
     mpImplData->mbPapersizeFromSetup = i_bPapersizeFromSetup;
     mpImplData->mxPrinter->SetPrinterSettingsPreferred( i_bPapersizeFromSetup );
     if ( i_bPapersizeFromSetup )
-        mpImplData->mbPapersizeFromUser = !i_bPapersizeFromSetup;
+    {
+        mpImplData->mbPapersizeFromUser = false;
+        mpImplData->mbOrientationFromUser = false;
+    }
 }
 
 bool PrinterController::getPapersizeFromSetup() const
@@ -1412,11 +1432,6 @@ bool PrinterController::getPapersizeFromSetup() const
     return mpImplData->mbPapersizeFromSetup;
 }
 
-Size& PrinterController::getPaperSizeSetup() const
-{
-    return mpImplData->maDefaultPageSize;
-}
-
 void PrinterController::setPaperSizeFromUser( Size i_aUserSize )
 {
     mpImplData->mbPapersizeFromUser = true;
@@ -1426,14 +1441,10 @@ void PrinterController::setPaperSizeFromUser( Size i_aUserSize )
     mpImplData->maUserPageSize = i_aUserSize;
 }
 
-Size& PrinterController::getPaperSizeFromUser() const
-{
-    return mpImplData->maUserPageSize;
-}
-
-bool PrinterController::isPaperSizeFromUser() const
+void PrinterController::setOrientationFromUser( Orientation eOrientation, bool set )
 {
-    return mpImplData->mbPapersizeFromUser;
+    mpImplData->mbOrientationFromUser = set;
+    mpImplData->meUserOrientation = eOrientation;
 }
 
 void PrinterController::setPrinterModified( bool i_bPrinterModified )
diff --git a/vcl/source/window/printdlg.cxx b/vcl/source/window/printdlg.cxx
index 26629168afb6..1be11ca583e9 100644
--- a/vcl/source/window/printdlg.cxx
+++ b/vcl/source/window/printdlg.cxx
@@ -1024,35 +1024,12 @@ bool PrintDialog::hasOrientationChanged() const
         || (nOrientation == ORIENTATION_PORTRAIT && eOrientation == Orientation::Landscape);
 }
 
-// make sure paper size matches paper orientation
-void PrintDialog::checkPaperSize( Size& rPaperSize )
-{
-    Orientation eOrientation = maPController->getPrinter()->GetOrientation();
-    if ( (eOrientation == Orientation::Portrait && rPaperSize.Width() > rPaperSize.Height()) ||
-         (eOrientation == Orientation::Landscape && rPaperSize.Width() < rPaperSize.Height()) )
-    {
-        // coverity[swapped-arguments : FALSE] - this is in the correct order
-        rPaperSize = Size( rPaperSize.Height(), rPaperSize.Width() );
-    }
-}
-
 // Always use this function to set paper orientation to make sure everything behaves well
-void PrintDialog::setPaperOrientation( Orientation eOrientation )
+void PrintDialog::setPaperOrientation( Orientation eOrientation, bool fromUser )
 {
     VclPtr<Printer> aPrt( maPController->getPrinter() );
     aPrt->SetOrientation( eOrientation );
-
-    // check if it's necessary to swap width and height of paper
-    if ( maPController->isPaperSizeFromUser() )
-    {
-        Size& aPaperSize = maPController->getPaperSizeFromUser();
-        checkPaperSize( aPaperSize );
-    }
-    else if ( maPController->getPapersizeFromSetup() )
-    {
-        Size& aPaperSize = maPController->getPaperSizeSetup();
-        checkPaperSize( aPaperSize );
-    }
+    maPController->setOrientationFromUser( eOrientation, fromUser );
 }
 
 void PrintDialog::checkControlDependencies()
@@ -1173,12 +1150,12 @@ void PrintDialog::updateNup( bool i_bMayUseCache )
         if( aMultiSize.Width() > aMultiSize.Height() ) // fits better on landscape
         {
             aMPS.aPaperSize = maNupLandscapeSize;
-            setPaperOrientation( Orientation::Landscape );
+            setPaperOrientation( Orientation::Landscape, false );
         }
         else
         {
             aMPS.aPaperSize = maNupPortraitSize;
-            setPaperOrientation( Orientation::Portrait );
+            setPaperOrientation( Orientation::Portrait, false );
         }
     }
 
@@ -2000,7 +1977,7 @@ IMPL_LINK( PrintDialog, SelectHdl, weld::ComboBox&, rBox, void )
     {
         int nOrientation = mxOrientationBox->get_active();
         if ( nOrientation != ORIENTATION_AUTOMATIC )
-            setPaperOrientation( static_cast<Orientation>( nOrientation - 1 ) );
+            setPaperOrientation( static_cast<Orientation>( nOrientation - 1 ), true );
 
         updateNup( false );
     }
@@ -2026,9 +2003,7 @@ IMPL_LINK( PrintDialog, SelectHdl, weld::ComboBox&, rBox, void )
         else
             aPrt->SetPaper( mePaper );
 
-        Size aPaperSize( aInfo.getWidth(), aInfo.getHeight() );
-        checkPaperSize( aPaperSize );
-        maPController->setPaperSizeFromUser( aPaperSize );
+        maPController->setPaperSizeFromUser( Size( aInfo.getWidth(), aInfo.getHeight() ) );
 
         maUpdatePreviewIdle.Start();
     }