Closed
Bug 275387
Opened 20 years ago
Closed 19 years ago
Print Preview Scale and orientation not adjusted (stuck on page setup settings)
Categories
(Core :: Print Preview, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta5
People
(Reporter: Obrie572, Assigned: masayuki)
References
()
Details
(Keywords: fixed1.8, regression)
Attachments
(1 file, 3 obsolete files)
3.82 KB,
patch
|
mconnor
:
review+
mtschrep
:
approval1.8b5+
|
Details | Diff | Splinter Review |
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•19 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•19 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
Closed: 19 years ago
Resolution: --- → DUPLICATE
Comment 3•19 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•19 years ago
|
||
we need to find a better owner for this. any suggestions?
Flags: blocking1.8b5? → blocking1.8b5+
Comment 5•19 years ago
|
||
This should probably be duped as bug 267422.
Comment 6•19 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•19 years ago
|
||
*** Bug 307267 has been marked as a duplicate of this bug. ***
Comment 8•19 years ago
|
||
*** Bug 307666 has been marked as a duplicate of this bug. ***
Comment 9•19 years ago
|
||
*** Bug 307754 has been marked as a duplicate of this bug. ***
Comment 10•19 years ago
|
||
(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•19 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?
Comment 12•19 years ago
|
||
Mike, see comment 11. /be
Comment 13•19 years ago
|
||
This seems massively broken on Windows, yes... Taking to investigate.
Assignee: roc → mconnor
Assignee | ||
Comment 15•19 years ago
|
||
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.
Assignee | ||
Comment 16•19 years ago
|
||
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
Assignee | ||
Comment 17•19 years ago
|
||
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.
Assignee | ||
Comment 18•19 years ago
|
||
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•19 years ago
|
Severity: normal → major
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
Comment 19•19 years ago
|
||
*** Bug 309570 has been marked as a duplicate of this bug. ***
Comment 20•19 years ago
|
||
*** Bug 309601 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Whiteboard: [needs review+SR roc]
Comment 21•19 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?
Assignee | ||
Comment 23•19 years ago
|
||
> 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).
Assignee | ||
Comment 24•19 years ago
|
||
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?
Comment 26•19 years ago
|
||
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•19 years ago
|
||
*** Bug 310416 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 28•19 years ago
|
||
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.
Assignee | ||
Comment 29•19 years ago
|
||
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.
Assignee | ||
Comment 30•19 years ago
|
||
(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.
Assignee | ||
Comment 32•19 years ago
|
||
O.K. I have a new idea. The new patch will come today or later.
Comment 33•19 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•19 years ago
|
Whiteboard: [needs review+SR roc]
Assignee | ||
Comment 35•19 years ago
|
||
I propose this approach. But I don't have much time for testing this patch. Please wait 12 - 15 hours.
Assignee | ||
Comment 36•19 years ago
|
||
Attachment #198102 -
Attachment is obsolete: true
Assignee | ||
Comment 37•19 years ago
|
||
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
Assignee | ||
Comment 38•19 years ago
|
||
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 39•19 years ago
|
||
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+
Assignee | ||
Comment 40•19 years ago
|
||
checked in to trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Attachment #198179 -
Flags: approval1.8b5?
Updated•19 years ago
|
Attachment #198179 -
Flags: approval1.8b5? → approval1.8b5+
Comment 42•19 years ago
|
||
*** Bug 324772 has been marked as a duplicate of this bug. ***
Comment 43•19 years ago
|
||
Still present in Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.1) Gecko/20060129 SeaMonkey/1.0 REOPEN
Comment 44•19 years ago
|
||
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.
Description
•