Closed
Bug 147605
Opened 23 years ago
Closed 22 years ago
Printing / Page settings reset themselves after print (no landscape printing)
Categories
(Core :: Printing: Output, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: r.davis, Assigned: roland.mainz)
References
Details
(Whiteboard: [ADT2])
Attachments
(1 file, 4 obsolete files)
No doubt this is related to Bugs 129033, 83750 among others: it concerns the persistence and effect of changes to paper orientation in Page Setup. Here's what happens to me using RC3 with KDE/RH7.2 1. Change Page Setup/Orientation to Landscape. Close dialog 2. Check Page Setup/Orientation. Shows Landscape selected as expected. 3. Open Print dialog. 4. Do nothing, just close Print dialog. 5. Check Page Setup/Orientation. Shows Portrait selected: wrong, surely? Alternatively: 4. Confirm print. (Printout will be Portrait orientation, not Landscape, as hoped!) 5. Check Page Setup/Orientation. Shows Portrait selected: surely wrong?
Assignee | ||
Comment 1•23 years ago
|
||
Confirming and taking myself. We are punished by the fact that we have to init nsIPrintSettings per printer but have (currently) no way to check whether the print settings were already initalised for this printer.
Assignee: rods → Roland.Mainz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•23 years ago
|
||
Accepting bug...
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1alpha
Assignee | ||
Comment 5•23 years ago
|
||
This patch now remembers the last printer used to initalise the |nsIPrintSettings| object and only writes into the object if the printer used for initalisation is not the same as the previous one used to init the object. However I think this is only a "workaround" - but I am not sure how a "better" solution would look like... I have to discuss this with rods. One final solution may look like this: Split |nsIPrintSettings| into an "application"-part (like "zoom" etc.) and a "device"-part (like "orientation", "paper name", "copies" etc.). When a user switches the selected printer both the "application"-part and "device"-part are saved to the prefs (before modifying the |nsIPrintAppSettings| and |nsIPrintDeviceSettings| objects) and the "device"-part is completely reinitalised based on the defaults for this printer device and prefs for this printer name. (This problem (bug 147605) would then we "fixed" because we save the prefs before switching the printer. Selecting the same printer as the previous one then just picks-up the previously saved settings).
Assignee | ||
Comment 6•23 years ago
|
||
This patch does the things more or less correctly: |InitPrintSettingFromPrinter()| now does the following: 1. Before writing to the |nsIPrintSettings| object the printer-specific attributes from the previous printer are written to the prefs (with the per-printer prefix) - see |SaveSettingsOfPreviousPrinter()| 2. Next step is to fetch the defaults from the device 3. Final step is to override the device defaults with per-printer prefs if there are any for this printer Comments: - |InitPrintSettingsFromPrefs()| and |InitPrintSettingsFromPrinter()| should return a flag field (with nsIPrintSettings::kInitSave* flags) indicating what has been "updated"/"touched" in the |nsIPrintSettings| object - There should be one |InitPrintSettingsFromPrinterAndPrefs()| which inits the nsIPrintSettings object with global prefs, device defaults and per-printer prefs ToDo: - Port the changes to the Xlib gfx code
Comment 9•23 years ago
|
||
I test patch 85318 on RedHat7.2 using "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020527". and on Solaris 8 using "Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.0.0+) Gecko/20020606". I meet the same problem on both platform. When I set "Landscape" printing mode, a progress bar came out when printing. But it made no progress and didn't print. When I click File->Print again, I got print error "Not Available". Roland, can you fix it if it's really a bug? BTW, Is patch 85318 a complete one or a demo version?
Assignee | ||
Comment 10•23 years ago
|
||
Louie Zhao wrote: > I meet the same problem on both platform. > When I set "Landscape" printing mode, a progress bar came out when printing. > But it made no progress and didn't print. AFAIK it did work for me (on a GTK+ Zilla, the patch does _not_ cover the Xlib code yet) ... but that was a long time ago... > When I click File->Print again, I got print error "Not Available". Well, either we leak the print device context or it was still "busy"... > Roland, can you fix it if it's really a bug? Yes, I can fix it... hopefully after wednesday... > BTW, Is patch 85318 a complete one or a demo version? See comment #6, sections "comments" and "todo"... that's what we need... Having per-printer nsIPrintSettings objects may be a better idea, but I am not sure whether rods likes that idea... The current patch only overwrites the print settings with the printer's defaults if we choose another printer... Mhhh... I have to chat with rods and discuss the fix with him.
Assignee | ||
Comment 11•23 years ago
|
||
[reminder (for me)] I discussed the issue with rods few secs ago, he said "go" for the creation of the per-printer nsPrintSettings patch...
Comment 14•23 years ago
|
||
hi, gisburn, I found 1.0 branch has also this bug, how is your xlib gfx related code going on now, any plan about trunk and branch for this bug? thanks.
Comment 15•23 years ago
|
||
Roland: your patch only works when you print one page, when the document is larger than 1 page, you will meet with the same problem as Louie' comment #9.
Assignee | ||
Updated•23 years ago
|
Attachment #85318 -
Flags: needs-work+
Assignee | ||
Updated•23 years ago
|
Attachment #85279 -
Flags: needs-work+
Assignee | ||
Comment 17•23 years ago
|
||
Another test patch (mainly for reference that the model of having global, per-printer nsIPrintSettings objects will not work since we would have to change frozen interfaces (hurray! ;-( ). I am getting grey hair with this print settings stuff (and noone pays me for the pain... ;-( ) ...
Assignee | ||
Comment 18•23 years ago
|
||
OK, new strategy, learning from my previous failure: I am going to cut nsIPrintSettings into two pieces, one application-specific part (|nsIPrintSettings|) and one device-specific part (|nsIPrintDeviceSettings|). We will still have one global |nsIPrintSettings| (therefore we won't need to change any frozen interfaces) but multiple per-printer |nsIPrintDeviceSettings| objects. Advantage of this design: - We are fixing this bug - We are getting a clean appoach for per-printer settings (since we're seperating application settings like shrink-to-fit from device settings (like device resolution, paper size, page orientation, etc.)). - We do not need to touch frozen interfaces [hopefully, but I am not going to gurantee this now before I have a working patch... :) ] Disadvantage: - Lots of work.... ;-( - It may require discussion which attributes are device specific and which are application specific.
Target Milestone: mozilla1.1alpha → mozilla1.1beta
Comment 19•23 years ago
|
||
Roland, since this bug has very high visibility and we want to have it fixed in coming Netscape7.0, I have to kick your butt again. How is your implementation of dividing interface into two part? do you need help? As I know, The schedule of Netscape7.0 is very urgent, according to the plan, today is the branch date, and code freezing date is very soon, Sun's schedule is very close with Netscape's schedule, I noticed that the target milestone of this bug is mozilla1.1 beta, is it possible? Can I recommend three alternative solution? 1 checkin your current patch which has global printing setting into trunk and branch, in order that Netscape7.0 can print using Lanscape mode. and at the same time open new bugs to enhance printing setting, such as multiple per-printer setting, store user's printing setup perference, etc. 2 If you can finish these "a lot of work" by yourself before Netscape7.0's code freezing, let you get your hair gray to white without payment:-) 3 Have us or others help you on the implementation of your new strategy. Since the Netscape7.0 schedule is urgent, I prefer option 1, What is your idea? Roland? and What is your idea, Rods?
![]() |
||
Updated•23 years ago
|
Comment 21•23 years ago
|
||
Thanks Boris for nominating this bugs as showstopper candidate, I also think this is a show stopper, but I do not know what rods, dcone and Roland are thinking about it. and I do not know how it is going on there.:-( any comments? thanks.
Assignee | ||
Comment 22•23 years ago
|
||
Jay Yan wrote: > Roland, since this bug has very high visibility and we want to have it fixed > in coming Netscape7.0, I have to kick your butt again. > How is your implementation of dividing interface into two part? do you need > help? As I know, The schedule of Netscape7.0 is very urgent, according to the > plan, today is the branch date, and code freezing date is very soon, Sun's > schedule is very close with Netscape's schedule, ;-( > I noticed that the target milestone of this bug is mozilla1.1 beta, is it > possible? Not really. 1.1beta is out. > Can I recommend three alternative solution? > > 1 checkin your current patch which has global printing setting into trunk and > branch, in order that Netscape7.0 can print using Lanscape mode. and at the same > time open new bugs to enhance printing setting, such as multiple per-printer > setting, store user's printing setup perference, etc. > > 2 If you can finish these "a lot of work" by yourself before Netscape7.0's code > freezing, let you get your hair gray to white without payment:-) Well, I guess I have a workaround (I am not sure whether it will work but I can file the patch). > 3 Have us or others help you on the implementation of your new strategy. > > Since the Netscape7.0 schedule is urgent, I prefer option 1, > > What is your idea? Roland? My problem is that I do not want to modify the frozen interfaces in nsIWebBrowserPrint&co. (I am sure some people will cut my head if I start to do this) The problem here is very simple: We init the nsIPrintSettings object each time we change the printer. The problem: We overwrite user selections with this initalisation (which is unavoidable, otherwise are breaking existing functionality elsewhere). AFAIK one of the ideal solutions would look like this: - We have multiple methods to init a nsIPrintSettings object (InitPrintSettingsFromPrefs(), InitPrintSettingsFromPrinter()) etc. but we _must_ only expose _one_ function - to obtain a fully initalised |nsIPrintSettings| object (which is only valid for the given printer): |nsIPrintSettings *GetNewPrintSettingsObjectForPrinter( const char *printerName)| (all the InitPrintSettings*-methods should be private and not accessible to embeddors or javascript). - nsIPrintSettingsService would provide an API to manage a list of these per-printer nsIPrintSettings objects (store such an object and find it later via the printer name). - |nsIPrintSettings| gets a new operator to copy certain fields from between two |nsIPrintSettings| objects Splitting the |nsIPrintSettings| object into an application-spefic part and a device-specific part would be nice, too.
Severity: normal → critical
Priority: -- → P3
Target Milestone: mozilla1.1beta → mozilla1.2alpha
Assignee | ||
Comment 23•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Attachment #85279 -
Attachment is obsolete: true
Attachment #85318 -
Attachment is obsolete: true
Attachment #90400 -
Attachment is obsolete: true
Assignee | ||
Comment 24•23 years ago
|
||
The workaround fixes the "can't print landscape"-issue, but IMHO we should file a bug to get the API fixed to avoid that anyone hits the same issue in the future. Known bugs of the workaround: - The workaround will not work in the scenario where a user selects "landscape" and then changes the printer name (changing the printer name will always reset the printsettings object).
Assignee | ||
Comment 25•23 years ago
|
||
Requesting r=/sr=/a{trunk}= etc. Lets test it a week on "trunk" first before we apply this to the 1.0.1 branch, please...
Comment 26•23 years ago
|
||
Comment on attachment 92671 [details] [diff] [review] Workaround forces that printdialog.js will init the global printsettings object only if we change the printer name >+ printService.initPrintSettingsFromPrefs(gPrintSettings, true, flags); >+ >+ dump("setPrinterDefaultsForSelectedPrinter: init\n"); >+ } >+ else >+ { this line should use this style, just same as other place in this file. if (foo) { } else { } And I'm not sure dump is necessary since we don't need them in runtime. Other than above r=pete
Attachment #92671 -
Flags: review+
Comment 28•23 years ago
|
||
Comment on attachment 92674 [details] [diff] [review] New workaround for 2002-07-18-08-trunk per petez's review comment which forces that printdialog.js will init the global printsettings object only if we change the printer name sr=jst
Attachment #92674 -
Flags: superreview+
Comment 29•23 years ago
|
||
kevin: can you take a look at this one, and add an ADT impact to the status whiteboard? thanks!
Updated•23 years ago
|
Whiteboard: [ADT2]
Comment 31•22 years ago
|
||
Comment on attachment 92674 [details] [diff] [review] New workaround for 2002-07-18-08-trunk per petez's review comment which forces that printdialog.js will init the global printsettings object only if we change the printer name r=pete ok, we just need request approval for this bug.
Attachment #92674 -
Flags: review+
Comment 32•22 years ago
|
||
Comment on attachment 92674 [details] [diff] [review] New workaround for 2002-07-18-08-trunk per petez's review comment which forces that printdialog.js will init the global printsettings object only if we change the printer name a=asa (on behalf of drivers) for checkin to 1.1
Attachment #92674 -
Flags: approval+
Comment 33•22 years ago
|
||
checked in trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 34•22 years ago
|
||
Thanks, Roland and Pete. Since it is a showstopper for 7.0, I am nominating this patch to be checked into OEM_BRANCH, mark it as BranchOEM. Jim, please help to approve it. thanks a lot. (BTW, who can tell me the meaning of "[ADT2]" in Whiteboard? thanks.)
Whiteboard: [ADT2] → [ADT2], BranchOEM
Comment 35•22 years ago
|
||
Checked in to oembranch.(a= Jim) Jim: Sorry for failing to write cvs log becasue of a typo. http://bonsai.mozilla.org/cvsquery.cgi?module=allrepositories&branch=&dir=&file=&who=jay.yan%25sun.com&sortby=Date&hours=2&date=week
Whiteboard: [ADT2], branchOEM+ → [ADT2], branchOEM+,fixedOEM
Comment 37•22 years ago
|
||
verified on 8/16 linux trunk...someone else(either Jay/Pete) will have to verify on OEM branch.
Status: RESOLVED → VERIFIED
Comment 38•22 years ago
|
||
I can finally print out my page in landscape on my 8/15 build of the OEM branch. Marked it verified for the OEM branch.
Whiteboard: [ADT2], branchOEM+,fixedOEM → [ADT2], branchOEM+, verifiedOEM
Assignee | ||
Comment 39•22 years ago
|
||
Unfortunately this patch regressed something - see bug 166217 ("Print settings on Linux are saved at shutdown but not read at next start") ... ;-(
Updated•22 years ago
|
Whiteboard: [ADT2], branchOEM+, verifiedOEM → [ADT2], verifiedOEM
Keywords: review → verifiedOEM
Whiteboard: [ADT2], verifiedOEM → [ADT2]
You need to log in
before you can comment on or make changes to this bug.
Description
•