Print Preview Scale and orientation not adjusted (stuck on page setup settings)

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
Print Preview
P1
major
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: Aaron Pfeifer, Assigned: masayuki)

Tracking

({fixed1.8, regression})

Trunk
mozilla1.8beta5
x86
Windows XP
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

13 years ago
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

12 years ago
confirmed on Windows Firefox trunk build 2005-05-18-07-trunk

no change in Scale works.  any change reverts back to "Shrink to fit"
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

12 years ago
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 ***
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE

Comment 3

12 years ago
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.

 
Status: RESOLVED → REOPENED
Flags: blocking1.8b5?
Resolution: DUPLICATE → ---
Summary: Print Preview Scale not adjusted when using custom scale below 10% the first time → Print Preview Scale and orientation not adjusted (stuck on page setup settings)

Comment 4

12 years ago
we need to find a better owner for this. any suggestions?
Flags: blocking1.8b5? → blocking1.8b5+

Comment 5

12 years ago
This should probably be duped as bug 267422.

Comment 6

12 years ago
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

12 years ago
*** Bug 307267 has been marked as a duplicate of this bug. ***
*** Bug 307666 has been marked as a duplicate of this bug. ***

Comment 9

12 years ago
*** Bug 307754 has been marked as a duplicate of this bug. ***
(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
Assignee: firefox → roc
Status: REOPENED → NEW
Component: General → Print Preview
Product: Firefox → Core
Version: unspecified → 1.0 Branch

Updated

12 years ago
Version: 1.0 Branch → Trunk
This is Windows only, other platforms don't have the UI. It sounds like a chrome
bug. Get mconnor to check?
Mike, see comment 11.

/be
This seems massively broken on Windows, yes...

Taking to investigate.
Assignee: roc → mconnor
This may be my regression. Please wait.
Keywords: regression
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.
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.
Depends on: 270079
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.
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.
Assignee: mconnor → masayuki
Status: NEW → ASSIGNED
Attachment #196706 - Flags: superreview?(roc)
Attachment #196706 - Flags: review?(roc)
(Assignee)

Updated

12 years ago
Severity: normal → major
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
*** Bug 309570 has been marked as a duplicate of this bug. ***

Comment 20

12 years ago
*** Bug 309601 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Whiteboard: [needs review+SR roc]

Comment 21

12 years ago
*** Bug 310013 has been marked as a duplicate of this bug. ***
(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?
> 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).
And see my comment 17.
(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?
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

12 years ago
*** Bug 310416 has been marked as a duplicate of this bug. ***
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.
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.
(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.

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.
O.K. I have a new idea. The new patch will come today or later.

Comment 33

12 years ago
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.
Attachment #196706 - Attachment is obsolete: true
Attachment #196706 - Flags: superreview?(roc)
Attachment #196706 - Flags: review?(roc)
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.
(Assignee)

Updated

12 years ago
Whiteboard: [needs review+SR roc]
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.
Created attachment 198140 [details] [diff] [review]
Patch
Attachment #198102 - Attachment is obsolete: true
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.
Attachment #198140 - Attachment is obsolete: true
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.
Attachment #198179 - Flags: review?(mconnor)
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
Attachment #198179 - Flags: review?(mconnor) → review+
checked in to trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #198179 - Flags: approval1.8b5?

Updated

12 years ago
Attachment #198179 - Flags: approval1.8b5? → approval1.8b5+
checked in to 1.8branch too.
Keywords: fixed1.8

Comment 42

11 years ago
*** Bug 324772 has been marked as a duplicate of this bug. ***

Comment 43

11 years ago
Still present in Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.1) Gecko/20060129 SeaMonkey/1.0

REOPEN
XPFE probably needs a port of this fix.  Please file a new bug against Seamonkey.
You need to log in before you can comment on or make changes to this bug.