Last Comment Bug 275387 - Print Preview Scale and orientation not adjusted (stuck on page setup settings)
: Print Preview Scale and orientation not adjusted (stuck on page setup settings)
Status: RESOLVED FIXED
: fixed1.8, regression
Product: Core
Classification: Components
Component: Print Preview (show other bugs)
: Trunk
: x86 Windows XP
: P1 major with 3 votes (vote)
: mozilla1.8beta5
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
: Jet Villegas (:jet)
Mentors:
http://www.google.com
: 307267 307666 307754 309570 309601 310013 310416 (view as bug list)
Depends on: 270079
Blocks:
  Show dependency treegraph
 
Reported: 2004-12-20 08:17 PST by Aaron Pfeifer
Modified: 2006-03-12 18:13 PST (History)
25 users (show)
asa: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
backing-out patch (1004 bytes, patch)
2005-09-19 15:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
Patch (3.81 KB, patch)
2005-09-30 21:08 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
Patch (7.77 KB, patch)
2005-10-01 09:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
quirk patch (3.82 KB, patch)
2005-10-01 17:25 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
mconnor: review+
mtschrep: approval1.8b5+
Details | Diff | Splinter Review

Description Aaron Pfeifer 2004-12-20 08:17:05 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

When using Print Preview, the initial Scale value is 100%.  If you try to
specify a custom scale below 10%, the value in the scale box will be changed to
30% (instead of the previous 100%),  but the actual scaled document is never
changed.

Reproducible: Always

Steps to Reproduce:
1. Visit http://www.google.com
2. Select Print Preview from the File menu
3. Drop-down the Scale combobox and select "Custom..."
4. Specify a value below 10% (not including 10%)

Actual Results:  
The value in the Scale combobox is changed to 30%, but the actual scaled
document remains unchanged.

Expected Results:  
Assuming that the correct behavior is using the minimum value, it should either
(1) Reflect the value in the Scale combobox or (2) Use the real minimum value,
which it seems is 10% in my test cases.
Comment 1 Tracy Walker [:tracy] 2005-05-18 11:20:49 PDT
confirmed on Windows Firefox trunk build 2005-05-18-07-trunk

no change in Scale works.  any change reverts back to "Shrink to fit"
Comment 2 Tracy Walker [:tracy] 2005-08-05 00:35:58 PDT
marking this as a dupe of 302495 because it has more info and is already flagged
to block 1.5

*** This bug has been marked as a duplicate of 302495 ***
Comment 3 Tracy Walker [:tracy] 2005-09-02 12:09:20 PDT
Going to the page setup dialog and changing settings works. That's covered and
WFM in bug 302495.  

However, the Scale and Orientation controls on the Print preview page still do
not function at all.  Landscape/Portrait buttons do not change orientation.
Scale menus has no affect. In fact anything selected just reverts to whatever is
set in the Page setup dialog.

reopening, nominating and morphing slightly to generalize the Scale and
Orientation tools failure.

 
Comment 4 Asa Dotzler [:asa] 2005-09-02 21:12:32 PDT
we need to find a better owner for this. any suggestions?
Comment 5 Adam Guthrie 2005-09-03 12:31:23 PDT
This should probably be duped as bug 267422.
Comment 6 Tracy Walker [:tracy] 2005-09-03 19:19:23 PDT
No, that bug occurs anytime a change in print preview settings is attempted,
whether it be from the main tools (broken - this bug) or from the Page setup
dialog (works - bug 302495).  If it tunrs out both bugs get fixed by a single
checkin, great.  But let's not lump them together just yet.  
   
Comment 7 Ali Chhotani 2005-09-06 16:18:09 PDT
*** Bug 307267 has been marked as a duplicate of this bug. ***
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-09-09 09:21:25 PDT
*** Bug 307666 has been marked as a duplicate of this bug. ***
Comment 9 Adam Guthrie 2005-09-09 23:42:23 PDT
*** Bug 307754 has been marked as a duplicate of this bug. ***
Comment 10 Brendan Eich [:brendan] 2005-09-14 14:15:04 PDT
(In reply to comment #5)
> This should probably be duped as bug 267422.

Maybe we shouldn't dup, but this bug is spectacularly miscategorized and
misassigned.  Roc, what can be done here?

/be
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-09-14 15:16:59 PDT
This is Windows only, other platforms don't have the UI. It sounds like a chrome
bug. Get mconnor to check?
Comment 12 Brendan Eich [:brendan] 2005-09-14 15:39:51 PDT
Mike, see comment 11.

/be
Comment 13 Mike Connor [:mconnor] 2005-09-14 17:50:58 PDT
This seems massively broken on Windows, yes...

Taking to investigate.
Comment 14 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-17 07:12:14 PDT
This may be my regression. Please wait.
Comment 15 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-17 08:16:36 PDT
My previous comment is wrong. This is not my regression.

I'm worried, see this code:
http://lxr.mozilla.org/mozilla/source/toolkit/components/printing/content/printPreviewBindings.xml#374
> 374           settings.shrinkToFit = false;
> 375           settings.scaling = aValue;
> 376           PrintUtils.savePrintSettings(settings);
> 377           PrintUtils.printPreview();

If user change the scaling, this code try to save these values. But in
printUtils, savePrintSettings save nothing data.

http://lxr.mozilla.org/mozilla/source/toolkit/components/printing/content/printUtils.js#132

PSSVC.savePrintSettingsToPrefs(aPrintSettings, false,
aPrintSettings.kInitSaveNativeData);

Currently we don't use kInitSaveNativeData. But I think we need to save
kInitSaveShrinkToFit and kInitSaveScaling to current printer prefs in this time
for next print preview.
Comment 16 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-19 13:53:40 PDT
I think that this is regression of bug 270079.
Bug 270079's patch is wrong. It breaks global printer settings.
Currently printer code does not assume that |GetGlobalPrintSettings| returns
latest printer settings.
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-19 13:54:48 PDT
Oops..

> Currently printer code does not assume that |GetGlobalPrintSettings| returns
latest printer settings.

right comment:
Currently printer code assumes that |GetGlobalPrintSettings| returns
latest printer settings.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-19 15:09:00 PDT
Created attachment 196706 [details] [diff] [review]
backing-out patch

We should back out the patch of bug 270079. I tested this patch, the print
preview's scale list box worked.

But we have another bug.
If changing scale, the content area is shrinked.
Comment 19 Phil Ringnalda (:philor) 2005-09-21 21:44:07 PDT
*** Bug 309570 has been marked as a duplicate of this bug. ***
Comment 20 Jo Hermans 2005-09-22 07:58:39 PDT
*** Bug 309601 has been marked as a duplicate of this bug. ***
Comment 21 Kevin Brosnan 2005-09-25 19:44:44 PDT
*** Bug 310013 has been marked as a duplicate of this bug. ***
Comment 22 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-09-26 19:47:20 PDT
(In reply to comment #15)
> If user change the scaling, this code try to save these values. But in
> printUtils, savePrintSettings save nothing data.
> 
>
http://lxr.mozilla.org/mozilla/source/toolkit/components/printing/content/printUtils.js#132
> 
> PSSVC.savePrintSettingsToPrefs(aPrintSettings, false,
> aPrintSettings.kInitSaveNativeData);
> 
> Currently we don't use kInitSaveNativeData. But I think we need to save
> kInitSaveShrinkToFit and kInitSaveScaling to current printer prefs in this time
> for next print preview.

This sounds right to me. Did you try it?
Comment 23 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-26 20:40:33 PDT
> This sounds right to me. Did you try it?

Yes, of course. But it is not better than current patch. Because we should not
save the prefs in print preview. The prefs should be written in printing.(I
think the users hope that the prefs should be saved with commiting time. I don't
think so for print preview. We should not save prefs in this time.)

Why you have granted to the patch of bug 270079? I think that we should not
destroy global printer object. Because we are using same instance of global
printer object in some places. Therefore, we have a problem in bug 270079. But
it is wrong way that global object's all field reset for some files of printer
object. Bug 270079 should change only the fields that has problem of global object.

I think bug 270079 made some regressions. Those are bug 308173 and a part of bug
118563(always reset the printing settings by every printing).
Comment 24 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-26 20:42:32 PDT
And see my comment 17.
Comment 25 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-09-27 16:34:12 PDT
(In reply to comment #23)
> Yes, of course. But it is not better than current patch. Because we should not
> save the prefs in print preview. The prefs should be written in printing.(I
> think the users hope that the prefs should be saved with commiting time. I
> don't think so for print preview. We should not save prefs in this time.)

I don't agree. I think it's fine to save prefs from the settings in print
preview. Can you give me the patch for that?
Comment 26 Mike Connor [:mconnor] 2005-09-28 08:24:10 PDT
We should absolutely save prefs in the print preview window, can we get traction
on that today?  I'll certainly review that ASAP.
Comment 27 Kevin Brosnan 2005-09-29 01:38:26 PDT
*** Bug 310416 has been marked as a duplicate of this bug. ***
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-29 09:26:49 PDT
I think that we should not save prefs in PrintPreview.
Because we must not change prefs if "print.save_print_settings" is false.
Currently, if "print.save_print_settings" is false, the global printer object is
always reset by every printing and print preview. This is not good behavior.

I think you are wrong, roc. We should have global printer object that is not
related prefs. But the global printer object is broken by bug 270079.
Comment 29 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-29 09:34:27 PDT
And if bug 270079 is correct, we cannot solve following paradox.

1. We should not save prefs if "print.save_print_settings" is false.
2. We need to change prefs for changing scale or orientation in print preview.

If we don't have global object, we need to save prefs in 2. But if
"print.save_print_settings" is false, we need to ignore
"print.save_print_settings". I.e, "print.save_print_settings" cannot control
saving print setteings.

However, if global object is not broken, we can solve this paradox.

1. -> Don't save if "print.save_print_settings" is false.
2. -> Don't save prefs, but we can change in global object and the global object
is refered in every print preview.
Comment 30 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-29 09:37:54 PDT
(In reply to comment #26)
> We should absolutely save prefs in the print preview window,

We don't save the prefs in Firefox 1.0 in print preview window. Firefox 1.0 is
only changing global object, not saving prefs.

Comment 31 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-09-29 15:29:21 PDT
print.save_print_settings is true by default and there is no UI for it. If it
causes any problems, we should just remove/ignore the pref. Let's go with the
patch for saving prefs, at least for branch. We can have a longer discussion
about what to do on trunk.
Comment 32 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-30 09:07:09 PDT
O.K. I have a new idea. The new patch will come today or later.
Comment 33 Scott MacGregor 2005-09-30 15:28:26 PDT
Comment on attachment 196706 [details] [diff] [review]
backing-out patch

obsoleting this patch to help improve our release bug queries so we don't think
we have a patch for this.

Time is running short if we are going to get this in. Having a new patch today
would improve the chances of this getting fixed in time.
Comment 34 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-09-30 16:02:02 PDT
This is pretty serious. I would think hard before releasing with this bug. Some
UI in the print preview window simply doesn't work, as I understand it.
Comment 35 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-09-30 21:08:59 PDT
Created attachment 198102 [details] [diff] [review]
Patch

I propose this approach.
But I don't have much time for testing this patch.
Please wait 12 - 15 hours.
Comment 36 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-01 09:51:46 PDT
Created attachment 198140 [details] [diff] [review]
Patch
Comment 37 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-01 14:39:12 PDT
Comment on attachment 198140 [details] [diff] [review]
Patch

This patch doesn't work fine on Thunderbird. Because Thunderbird is using only
a part of printUtils. I'll try next.
Comment 38 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-01 17:25:51 PDT
Created attachment 198179 [details] [diff] [review]
quirk patch

This patch fixes regression.

But I trust that this patch is incorrect. Because we should be able to get
latest printer settings by nsPrintOptions::GetGlobalPrintSettings. I cannot
understand why we have nsPrintOptions::GetGlobalPrintSettings and
nsPrintOptions::GetNewPrintSettings. These methods are same.

If roc is correct, we need refactoring for printUtils.js in toolkit.
Comment 39 Mike Connor [:mconnor] 2005-10-02 21:28:51 PDT
Comment on attachment 198179 [details] [diff] [review]
quirk patch

yeah, toolkit's printing impl needs love, let's go with this for now, and
figure out what we need to whack on trunk
Comment 40 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-03 09:22:28 PDT
checked in to trunk
Comment 41 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-03 09:33:32 PDT
checked in to 1.8branch too.
Comment 42 Andrew Schultz 2006-01-27 20:10:25 PST
*** Bug 324772 has been marked as a duplicate of this bug. ***
Comment 43 tech 2006-01-30 03:21:06 PST
Still present in Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.1) Gecko/20060129 SeaMonkey/1.0

REOPEN
Comment 44 Mike Connor [:mconnor] 2006-01-31 06:46:34 PST
XPFE probably needs a port of this fix.  Please file a new bug against Seamonkey.

Note You need to log in before you can comment on or make changes to this bug.