'Page Setup' on the Mac File menu used to bring up the native printer page setup dialog. It now does nothing.
File > Page Setup still comes up on linux. is this a problem on win32 and/or mac os x?
s/mac os x/mac os 9.x
OS: Mac System 9.x → MacOS X
confirming on MacOS 9.x with 1/7 commercial build also....Page Setup doesn't bring any dialog up.
over to don I am not sure how it worked before, but I am sure my checkin probably overrode something.
Assignee: rods → dcone
Created attachment 65430 [details] [diff] [review] Patch to implement the PrintSettings dialog thru the PrintOptions interface.
1) Put a comment above the new includes in nsPrintOptionsImpl.cpp 2) You may want to remove you ifdef NOT_NOW before checkin. 3) Fix the spacing in the makefile.win r=rods
Does this show the XP print setup dialog, or the Mac printer driver page setup dialog? This bug was filed specifically on the latter, since it's the only way to get to some printer-specific options on Mac.
This will show the Mac native dialog, the Mac overrides the xplatform dialog.
Comment on attachment 65430 [details] [diff] [review] Patch to implement the PrintSettings dialog thru the PrintOptions interface. sr=attinasi
Attachment #65430 - Flags: superreview+
will this land for 0.9.8?
OS: MacOS X → All
*** Bug 120707 has been marked as a duplicate of this bug. ***
latest patch looks ok to me; r=brade
Does the patch fix OS X too?
I dont think OS X ever had print settings working, I have a bug for that. I will look at that next.
Sure it did; I fixed them at the same time I fixed OS 9.
Created attachment 66188 [details] [diff] [review] Had a small bug.. fixed and tested on the Mac & Windows
please fix the comment in utilityOverlay.js r=brade
add the read/write prefs calls to the OSX file and then r=brade
Created attachment 66310 [details] [diff] [review] Added the Read and Write prefs for OS X stuff.
Comment on attachment 66310 [details] [diff] [review] Added the Read and Write prefs for OS X stuff. r=brade
Attachment #66310 - Flags: review+
Comment on attachment 66310 [details] [diff] [review] Added the Read and Write prefs for OS X stuff. Comments: REQUIRES= xpcom \ + windowwatcher \ + dom \ This scares me, greatly (see below). +nsPrintOptions::ShowPrintSetupDialog(nsIPrintSettings *aPS) ... + rv = wwatch->OpenWindow(parent, "chrome://communicator/content/printPageSetup.xul", + "_blank", "chrome,modal,centerscreen", array, + getter_AddRefs(newWindow)); I think that this is the wrong way to go about this. gfx is a low-level DLL, and as such should not be doing things like bringing up XUL windows (think about embedders). It should also not have dependencies on DOM and WindowWatcher. If you have to throw a XUL dialogs, this should be done from JS code, and the JS that runs the dialog should interact with the gfx interfaces for print settings. I really don't think that this code should be here.
sfraser: gfx/src/gtk/ and other platforms use windowwatcher/dom since eternity, too - and noone complained about that yet...
The problem is.. this is needed for the .98 build to get the dialogs working.. Those includes for the Makefile are also dependencies in the GFX/SRC/Windows directories.. so your concerns.. may be valid.. but its a bigger problem than just me using them. That problem can be addressed later.. I would hope.. but in the mean time.. this is the only way to get the print settings dialogs working again.. for the .98 release.
What is needed for 0.9.8? Just the Mac page setup dialog fixes, or the Windows changes too?
Those dependencies that I used are not new to GFX, so inorder to fix what your saying.. it has to be fixed a bunch of other places. The Mac dialogs do not work right now.. and this is the only way to fix it.. by redoing how all the platforms put up the dialog.
Simon--the changes in xpfe/communicator/resources/content/mac/platformCommunicatorOverlay.xul would be enough to show the XP dialog (it's pretty bad; no window title bar, etc.). Also, Don and I have discussed an additional change to the above file to handle the print dropdown list (on the toolbar) as well. He will make the same change twice in that file (File menu is changed in existing patch, now he'll also change dropdown list for toolbar).
I don't see why a small reworking to bring up the JS from XUL, rather than the gfx code, is hard. None of the other code has to change, right?
ShowPrintSetupDialog is the backstop.. all platforms will bring that up if there in not a native implementation. If I reworked it.. the xul and js for each platform would have to be different.. and call different routines.. right? The ShowXPPrintDialog is itself dependent on those routines, or files. I think that the patch is the clean way to solve the problem of the printsettings dialog, and then the next problem that your worried about is breaking that dependency. Reworking to bring up the JS from XUL does not break this dependency, it just re-arranges it. GFX the way is stands now.. depends on those files.. and this patch does not add to it.. it just keeps it the same. So I think that putting this patch in.. then solving the problem of how to break that dependency is the correct thing to do. The patch has a single place it calls to bring up a dialog.. and each platform can override this.. I think thats a great way to do it. I did not add a dependency.. it will be there no matter if this patch goes in or not.
> ShowPrintSetupDialog is the backstop.. all platforms will bring that up if there > in not a native implementation. If I reworked it.. the xul and js for each > platform would have to be different.. and call different routines.. right? I think what you need is a method on nsIPrintOptions that is "HasNativePageSetupDialog" or something. Then your JS can look like: if (printOptionsService.hasNativePageSetupDialog) printOptionsService.ShowPrintSetupDialog(printSettings); else window.openDialog(parent, "chrome://communicator/content/ printPageSetup.xul"........) [note that your .idl should use leading lowercase method names, so this should be printOptionsService.showPrintSetupDialog(printSettings)] > The ShowXPPrintDialog is itself dependent on those routines, or files. I think > that the patch is the clean way to solve the problem of the printsettings > dialog, and then the next problem that your worried about is breaking that > dependency. Reworking to bring up the JS from XUL does not break this > dependency, it just re-arranges it. How so? You're making the gfx code require windowwatcher and dom header files at build time, and DLLs at runtime (to be functional). That's made explicit by this change: REQUIRES= xpcom \ + windowwatcher \ + dom \ string \ It also hard-codes in C++ a url to a chrome file, thus making assumptions about the structure of JAR files. Moving the dialog invocation into C++ avoids all these problems. > GFX the way is stands now.. depends on > those files.. and this patch does not add to it.. it just keeps it the same. Not true, as I point out above. > So I think that putting this patch in.. then solving the problem of how to break > that dependency is the correct thing to do. > > The patch has a single place it calls to bring up a dialog.. and each platform > can override this.. I think thats a great way to do it. I did not add a > dependency.. it will be there no matter if this patch goes in or not. But you added to the REQUIRES in the makefile, thus adding a dependency! That's what REQUIRES is all about.
There is a routine.. in gfx/src/windows.. that makefile also requires those fields.. hence GFX requires the same files.. All is did is promote that routine in the gfx/src/windows directory to the gfx/src directory. That other routine.. will disapear.. when rods determines it is no longer needed because I moved it to the GFX/src nsPrintOptionsImp class. The other file it is in.. that requires those same files is gfx/src/windows/nsDeviceContextSpecWin. So I did not add depedencies to gfx.. just moved those same dependecies up one directory. I did not make these dependencies.. they are all over.. and GFX has those same dependencies all over. If I actually did something.. or added a dependecy that was not there.. I would agree.. but I did not. I would agree these (all the dependecies) should be cleared up.. but that does not mean that this patch is incorrect.. I think its the right way.. I just think that the windowwatcher is the only way to get some functionality that is needed.. And that will not go away even if I redo the patch the way you suggest. That dependecy is STILL there.. is just in the gfx/src/windows directory instead of the grx/src directory. That does not include the other dependecies in the gfx/src/gtk directory.. and I would bet.. there are plenty more instances of this kind of thing. Dont get me wrong.. I agree.. these have to be fixed.. but the current way that nsPrintOptions and nsPrintSettings work.. those dependencies will not go away, they are there currently.. I just moved it a directory higher.
Just to make things more clear.. all I really did.. was move some functionality in GFX up a directory level from the platforms to the Impl.. I did not add.. I did that so I would be able to expose the functions needed to put up the dialog settings on Mac. To rid GFX of those dependecies is not the goal of this bug.. and I did not add those dependencies.. those were there already.. just in a different spot.. but still there.
What if we checked this patch into 0.9.8, and do the right thing on the trunk?
I will take a sr for the branch only.. and look into figuring out how to get rid of that dependency.. but remember.. this patch does not cause that dependency.. the Checkin Rods did a month ago.. created it. I still think this patch accomplishes the task of xplatform print settings.. and overiding xp with native dialogs the correct way from an engineering standpoint.. and we need to solve the dependency issue another way. But I will take what I can get for .98 :) So please do sr.. it for .98.. and I will work with you getting a solution we both like for the trunk.
Comment on attachment 66310 [details] [diff] [review] Added the Read and Write prefs for OS X stuff. sr=sfraser for 0.9.8 branch checkin only.
Attachment #66310 - Flags: superreview+
I think we should create a separate bug to cleanup the dependencies that are created by the cross platform print dialog. Regardless of where the dependecies are created (gfx/src as ocurs in this patch) or in both (gfx/src/windows, gfx/src/gtk) as it is currently on the trunk, we are in a sitution where the gfx module can not be built without these other modules. Getting the functionality in PageSetup working for as many user's as possible is critical right now, so we can get additional testing. I don't think we should hold up getting this fix in until the dependencies have been removed. I think the depedency bug should have a high priority however, since too many dependencies have crept into gfx over time and adding a couple of more just compounds the problem and sends the wrong message.
a=asa (on behalf of drivers) for checkin to 0.9.8
checked into the .98 branch
removing keyword. fix verified on the branch.
I have talked with Rod and Kevin about this issue. That dependency.. was introduced by Rods checkin a month earlier. He has a bug.. http://bugzilla.mozilla.org/show_bug.cgi?id=115136 that addresses that issue, and the way we bring up the XP dialog will not change. So.. I think this patch should be allowed to be checked in on the trunk and that dependency issue should be fixed by bug 115136. If I dont check this in..the Mac print settings will be broken.. and that dependency will still live on in the gfx/src/windows directory.. basically a lose lose situation. I could come up with a patch that just puts up the Mac dialog..and jump through alot of hoops.. but that dependency would still be there.. and.. I would have added some code that is not the proper xp way to bring up Native or XP print settings dialogs.
I'd like to hear about the solution to bug 115136 before allowing this patch in on the trunk.
That solution is for Rod. I guess print settings wont work for a while on the Mac.
Windows ME. trying to change the aspect of the printer page. File/Page Setup/Header Right: it is impossible (for me) to write anything there.
Comment on attachment 66310 [details] [diff] [review] Added the Read and Write prefs for OS X stuff. OK, let's get this in on the trunk, and make sure that 115136 removes the bogus dependencies.
Marking nsbeta1+. Setting milestone to Moz0.9.9
Keywords: nsbeta1 → nsbeta1+
Target Milestone: --- → mozilla0.9.9
fixed and checked in.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
okay it works now. verified using 2/13 trunk build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.