546270 - Firefox Javascript anomaly - landscape print orientation reverts to portrait

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Matěj Cepl]

Description of problem:
When setting the print orientation within Firefox to landscape and then
invoking additional Firefox windows (within the same process), we are observing
that the print orientation reverts back to portrait.

How reproducible:
always

Version where reproduced:
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.3a2pre) Gecko/20100214 Minefield/3.7a2pre

Steps to Reproduce:
1.  Start Firefox.
2.  Run a Javascript application
3.  In Page Setup, set print orientation to landscape.
4.  Via Javascript program, invoke a new window. Use window.open() function.
5.  In new window (first sub-window), open yet another window using the
window.open() function.
6.  In the newest window (second sub-window), Press Ctrl-P
to open the print dialog window.
7.  In print dialog window, select Page Setup tab. It will show a portrait
outline of the page.  If you actually print, it comes out in Portrait mode.

Reproducer is on http://www.tizag.com/javascriptT/javascriptprint.php

Actual results:
Print orientation reverts to Portrait without asking for Portrait.

Expected results:
Print orientation remains landscape for the print.

Additional info:
Another issue is that A4 paper (set in File/Page Setup) is ignored, and the print goes to US Letter paper.

Could be related to bug 156992

Temporary workaround seems to be to set "print.print_orientation" to 1, from about:config.

--------------------------------------
Some more comments on the bug:

I think the portrait/landscape setting is not applied because the printer name
is not found in the case of the window.print() function. 

When using the browser print menu, both queues "printer" and
"PostScript/default" are used, with the latter is the one for which the
expected orientation is set.

When using window.print() function, the "PostScript/default" is not used.

In nsPrintOptions::InitPrintSettingsFromPrefs():

    NS_IMETHODIMP
    nsPrintOptions::InitPrintSettingsFromPrefs(nsIPrintSettings* aPS,
                                              PRBool aUsePNP, PRUint32 aFlags)
    {
     [...]

     nsAutoString prtName;
     // read any non printer specific prefs
     // with empty printer name
     nsresult rv = ReadPrefs(aPS, prtName, aFlags);          <==== 1st call
     NS_ENSURE_SUCCESS(rv, rv);

     // Get the Printer Name from the PrintSettings
     // to use as a prefix for Pref Names
     rv = GetAdjustedPrinterName(aPS, aUsePNP, prtName);
     NS_ENSURE_SUCCESS(rv, rv);

     if (prtName.IsEmpty()) {                                <==== Here
       NS_WARNING("Caller should supply a printer name.");
       return NS_OK;
     }

     // Now read any printer specific prefs
     rv = ReadPrefs(aPS, prtName, aFlags);                   <==== 2nd call
     if (NS_SUCCEEDED(rv))
       aPS->SetIsInitializedFromPrefs(PR_TRUE);

     return NS_OK;
    }

So the problem is that GetAdjustedPrinterName() returns an empty name in the
case of window.print() (while it returns "PostScript/default" when printing
from the regular browser menu)

This is from nsPrintSettingsGTK::GetPrinterName():

    NS_IMETHODIMP
    nsPrintSettingsGTK::GetPrinterName(PRUnichar * *aPrinter)
    {
     const char* gtkPrintName = gtk_print_settings_get_printer(mPrintSettings);
     if (!gtkPrintName) {
       if (GTK_IS_PRINTER(mGTKPrinter)) {
         gtkPrintName = gtk_printer_get_name(mGTKPrinter);
       } else {
         // This mimics what nsPrintSettingsImpl does when we try to Get before
we Set
         nsXPIDLString nullPrintName;
         *aPrinter = ToNewUnicode(nullPrintName);                   <==== Here
         return NS_OK;
       }
     }
     *aPrinter = ToNewUnicode(nsDependentCString(gtkPrintName));
     return NS_OK;
    }

mGTKPrinter is not set (null) when using the window.print() function whereas it
is set when using the regular print menu option, so that GetPrinterName() does
not return an empty name in that case. 

I also tried with Firefox 3.6 on Windows and there is not such problem as far
as I can see, so that seems to be specific to the gtk backend (but the print
dialog in Windows let the user change the orientation so the problem might not
be seen). So it might not be the same as the upstream bug mentioned.

Problem is I am not sure why mGTKPrinter is not set in the case of
window.print() :)    
Increasing severity/priority to match current issue-tracker status.

Comment 1

[Matěj Cepl]

Attachment: result of reproduction

Comment 2

[Olivier Fourdan]

Attachment: Possible patch

The mGTKPrinter is set in SetupSilentPrinting(), this patch is a possible fix.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 426977 [details] [diff] [review]
Possible patch

Can you check this patch please?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Hmm.  I'm not sure I'm qualified to review this.  roc, do you know who might be?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It makes sense to me. However, I think the call to SetupSilentPrinting should be wrapped in EnterModalState()/LeaveModalState().

Comment 6

[Olivier Fourdan]

Attachment: Updated patch

Updated patch based on comment #5

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 427090 [details] [diff] [review]
Updated patch

With the EnterModalState()/LeaveModalState()

Comment 8

[Dão Gottwald [:dao]]

The patch doesn't apply. nsGlobalWindow.cpp has moved.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Attachment: trunk version

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 431618 [details] [diff] [review]
trunk version

Can we have a review for the trunk version please?

Comment 11

[Daniel Holbert [:dholbert]]

Comment on attachment 431618 [details] [diff] [review]
trunk version

Patches are identical, aside from the new flattened nsGlobalWindow.cpp path.  I think you're fine to just carry forward the r=roc.

I'll land this on your behalf.

Comment 12

[Daniel Holbert [:dholbert]]

pushed: http://hg.mozilla.org/mozilla-central/rev/ef19608fe045

Comment 13

[Martin Stránský [:stransky] (ni? me)]

Great, Thanks.

Comment 14

[Martin Stránský [:stransky] (ni? me)]

Unfortunately the fix does not work properly. The printer settings is broken when there is missing a default printer. It happens when no printer is installed (i.e. print to file) or it's just unset.

Simple steps to reproduce the bug with latest trunk:

1) Remove all printers from system
2) Add one printer by CUPS, don't set is as default
3) Change page orientation (File->Page setup)
4) Press CTRL+P, the printer is selected and printing settings is correct
5) Open print dialog via. javascript applet. No printer is selected and print settings is incorrect. 

It's because of a difference between code in menu handler (print() function in printUtils.js) and print routine issued from web page (nsGlobalWindow::Print()). I think we should sync these two places.

Comment 15

[Martin Stránský [:stransky] (ni? me)]

Attachment: sync nsGlobalWindow::Print() with PrintUtils.js

The patch copies settings scheme from getPrintSettings() and setPrinterDefaultsForSelectedPrinter() from printUtils.js

Comment 16

[Martin Stránský [:stransky] (ni? me)]

Attachment: 1.9.2 patch

Comment 17

[Daniel Holbert [:dholbert]]

Landed the "sync nsGlobalWindow::Print() with PrintUtils.js" patch on m-c:
  http://hg.mozilla.org/mozilla-central/rev/d7fc936e9d85