127258 - Print Preview: changing scale also changes orientation from Landscape to Portrait

Status: VERIFIED DUPLICATE of bug 131852

(Core :: Print Preview, defect, P3)

Description

[rcummins]

w2k Build 2002022103

While in Print Preview, change the orientation to "Landscape".  Now change the
scale.  The previewed page changes orientation back to "Portrait" (which is
weird), but the state of the Portrait and Landscape buttons in the toolbar still
indicate that the page is being displayed Landscape (which is also weird).  If
you click the Landscape button again, the previewed page will again be displayed
as Landscape (which is expected).

Comment 1

[Boris Zbarsky [:bzbarsky]]

front-end bug

Comment 2

[Samir Gehani]

And why is this a front-end bug?  

Comment 3

[Samir Gehani]

As I suspected, the semantics of globalPrintSetings and globalPrinSettingsValues
have been unclear to me and are possibly the cause of this bug.

In the quoted stack ``tracelet'' below, the print preview dialog gets the
globalPrintSettings and tries to change the *scaling* attribute.  But every time
the GetGlobalPrintSettings() function is called it *initializes* the settings
from the printer.

...
nsPrintSettings::SetOrientation
nsDeviceContextSpecWin::SetPrintSettingsFromDevMode
nsPrinterEnumeratorWin::InitPrintSettingsFromPrinter
DocumentViewerImpl::InitPrintSettingsFromPrinter
DocumentViewerImpl::GetGlobalPrintSettings
XPTC_InvokeByIndex
...

Rod,
What should the print preview dialog be using?  Is it OK for the front end to
ask for the globalPrintSettings and keep manipulating them as users change
attributes like scaling and orientation while print previewing?  The
documentation is unclear to me.  

Or is it the case that the globalPrintSettings should be initialized only once
while we are in print preview mode?  I'll attach a patch that handles the latter
case which fixes this bug and prints with the correct orientation.  I don't know
if this is the correct solution though and need your input.  So please review or
suggest alternatives.  Thanks.

Comment 4

[Samir Gehani]

Attachment: Patch to ``fix'' backend

Only init globalPrintSettings once while in print preview mode.

Comment 5

[Samir Gehani]

CC'ing Boris.  See comment 2.  Thanks.

Comment 6

[Boris Zbarsky [:bzbarsky]]

My comment was predicated on the assumption of a clear and documented API.... :)

Comment 7

[Samir Gehani]

Attachment: Better patch

!mIsDoingPrintPreview really should have been mIsDoingPrintPreview in the last
patch.

Comment 8

[Samir Gehani]

Boris,
Never mind then.  I thought you had some insight and could educate me about the
semantics.  Thanks.

Comment 9

[Roland Mainz]

Is there a reason why we do not check for the return code of
InitPrintSettingsFromPrinter() ?

Comment 10

[Boris Zbarsky [:bzbarsky]]

*** Bug 128722 has been marked as a duplicate of this bug. ***

Comment 11

[Markus Långström]

*** Bug 128732 has been marked as a duplicate of this bug. ***

Comment 12

[Roland Mainz]

Other question:
Is it legal that the printer name argument of InitPrintSettingsFromPrinter() is
|nsnull| ? If yes - what should InitPrinttSettingsFromPrinter() do in this case
(currently it's a NO-OP on unix platforms, see
http://lxr.mozilla.org/seamonkey/source/gfx/src/gtk/nsDeviceContextSpecG.cpp#586
...) ?

Comment 13

[Peter Trudelle]

nsbeta1+ per Nav triage team

Comment 14

[Samir Gehani]

Rod,
Could you comment/review on the patch or take this bug if you think it can be
fixed in the backend and my patch is incorrect?  Thanks.

Comment 15

[Samir Gehani]

Rod says this is covered by a more thorough patch in bug 131852.

*** This bug has been marked as a duplicate of 131852 ***

Comment 16

[sujay]

verified.