Closed Bug 113917 Opened 23 years ago Closed 23 years ago

Need support for XP Print Dialog, or user -defined Print dialog

Categories

(Core :: Printing: Output, defect, P1)

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla0.9.8

People

(Reporter: rods, Assigned: rods)

References

Details

Attachments

(9 files, 18 obsolete files)

(deleted), text/html
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), application/zip
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
dbaron
: review+
Details | Diff | Splinter Review
Each platform needs infrastructure for being to "fly" a native dialog, or there own dialog.
Status: NEW → ASSIGNED
Priority: -- → P1
Summary: Need support for XP Dialog → Need support for XP Dialog
Target Milestone: --- → mozilla0.9.7
Summary: Need support for XP Dialog → Need support for XP Print Dialog, or user -defined Print dialog
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Attached file Printing Status (deleted) —
First draft of the work that needs to be done for printing to support this bug
Please review the attached document, a very large patch is coming today, covering all platforms.
Attached patch /mozilla/gfx/src/nsPrintSettingsImpl.h (obsolete) (deleted) — Splinter Review
(new file)
Attached patch /mozilla/gfx/src/nsPrintSettingsImpl.cpp (obsolete) (deleted) — Splinter Review
(new file)
(new file)
(new file)
(new file)
Attached file Zip file containing the directory of "htmldiff" files (obsolete) (deleted) —
The zip file contains all the diffs in an html format that enables you to browse the differences between what was on my disk and trunk. Much easier to review the changes than looking at the patch file.
Attached patch the patch file (330k) (obsolete) (deleted) — Splinter Review
This is the entire patch file. The Windows and GTK platforms have been compiled and tested. I need folks to compile and test: QT BEOS Xlib Mac (brade has volunteered) photon OS/2 (jblanco has volunteered)
To build: 1) Apply patch 2) Add the 5 attached files to the appropriate locations 3) build (hopefully it all builds and runs)
I applied the patch to another Windows build and it failed on the second to last file. Here is the file and then all you have to do is remove all the lines in the patch file up until the last file and re-apply it.
This patch break PrintPreview for some reason, I attach a new patch soon.
Comment on attachment 61546 [details] [diff] [review] the patch file (330k) The patch is fine except that PrintPreview doesn't work correctly. I will attach a new patch shortly
Attachment #61546 - Attachment is obsolete: true
The attached is a diff of gfx/src/os2 directory after I built mozilla with the applied changes. I made a couple changes for OS/2 to build. Changing settings in the Page Setup dialog doesn't affect printing on OS/2 though. I will need to work on that.
Blocks: 114953
Attachment #61474 - Attachment is obsolete: true
Attachment #61475 - Attachment is obsolete: true
Attachment #61476 - Attachment is obsolete: true
Attachment #61477 - Attachment is obsolete: true
Attachment #61478 - Attachment is obsolete: true
Attachment #61544 - Attachment is obsolete: true
cd to the root directory and unzip
Attachment #61772 - Attachment is obsolete: true
NOTE: If you are trying to build, the files in the zip file have been checked in (but aren't in the build until you apply the patch)
rods: I am going to implement a new class in bug 111934 which should represent a single printer (instance) which can be queried for attributes supported by this object. Can we discuss this somehow (IRC?) and sync out development ?
Attachment #61572 - Attachment is obsolete: true
Attachment #61794 - Attachment is obsolete: true
Attached patch Complete patch (includes print progress dialog) (obsolete) (deleted) — Splinter Review
This patch includes the OS/2 changes. All the new files are checked in, so if you pull, then apply this patch everything should build. I have built and tested on Linux and on Windows. dcone will be doing Mac.
Unzip, it creates a "diffs" directory. Load the index.html file and view the diffs file by file.
This patch does the follow: Much of what has changed in "Print Status" document attach to this bug, here is an overview. * Eliminates the nsIPrintListener interface, it now uses the nsIWebProgressListener * Implements Print Progress which is used by nsDocumentViewer, uses a pref as to whether to display the progress dialog * Implements the Page Setup dialog, this is all done in script, the values are set/get into the PrintSettings object * Uses nsIPrintSettings for all of printing and print preview * Reworked nsIPrintPreviewContext, it was not actually implementing nsIPrintPreviewContext (miracle anything worked) * Reduced the items in the printjogoptions dialog used by non-windows XP dialogs it overlapped with the page setup items * Windows can now use either a native dialog or an XPDIalog, it is controlled by a pref * Most of the changes to the various platforms in GFX was enabling them to get a nsIPrintSettings object down into the initialization of the DeviceContextSpec * On Windows the creation of the native print dialog was moved from the factory to the DeviceCOntextSpec. This makes it more consistent with the other platforms
This patch supercedes the patch for nsDeviceContextSpecWin.cpp in the large patch
Blocks: 112776
Blocks: 112803
r=mscott for the mailnews changes in the patch from 12/15/01 16:03 pm.
Attachment #61865 - Attachment is obsolete: true
Attachment #61867 - Attachment is obsolete: true
Attachment #61946 - Attachment is obsolete: true
Attached patch complete patch (again) (obsolete) (deleted) — Splinter Review
COmpiles on Windows and Linx, Mac will soon follow
Rod sent me the changes via email. I've reviewed the files in content, dom, gfx and webshell. Minor stuff below, considering this is such a HUGE patch, I'd even call these nits trivial. sr=attinasi for the reviewed files. nsDocumentViewer.cpp: * when casting, use NS_STATIC_CASE instead of old C-style casts * the iterators, like PrintData::DoOnProgressChange, should check or at least assert that the ElementAt is not null * what is the difference between mDebugFile, mDebugFD, and mDebugFilePtr? nsRuleNode.cpp: * why was the assertion comment-out? can it be removed, or at least explain why it is triggering now... nsPrintOptionsImpl.cpp: * so, we don't check return values for pref access?
r=dcone
Comment on attachment 62245 [details] [diff] [review] complete patch (again) Minor build issue with Xlib toolkit: -- snip -- nsDeviceContextSpecXlib.cpp Building deps for ../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/xl ib/nsDeviceContextSpecXlib.cpp /opt/SUNWspro/bin/CC -o nsDeviceContextSpecXlib.o -c -DOSTYPE=\"SunOS5\" -DOSARCH=\"SunOS\" -DOJI -DHAVE_XIE -D_IMPL_NS_GFXNONXP -DUSE_XPRINT -DUSE_MOZILLA_TYPES -I../../../dist/include/xpcom -I../../../dist/include/string -I../../../dist/include/xlibrgb -I../../../dist/include/widget -I../../../dist/include/dom -I../../../dist/include/layout -I../../../dist/include/content -I../../../dist/include/appshell -I../../../dist/include/js -I../../../dist/include/necko -I../../../dist/include/pref -I../../../dist/include/util -I../../../dist/include/view -I../../../dist/include/uconv -I../../../dist/include/locale -I../../../dist/include/unicharutil -I../../../dist/include/gfx2 -I../../../dist/include/imglib2 -I../../../dist/include/mozcomps -I../../../dist/include/windowwatcher -I../../../dist/include/intl -I../../../dist/include/gfx -I../../../dist/include -I/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib/dist/include/nspr -I../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/ xlib/../xprint -I../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/ xlib/. -I../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/ xlib/.. -I/usr/openwin/include -KPIC -I/usr/openwin/include -mt -DDEBUG -D_DEBUG -DDEBUG_mozilla -DTRACING -g -O -I/usr/openwin/include -I/usr/openwin/include -DMOZILLA_CLIENT -DBROKEN_QSORT=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_SYS_BYTEORDER_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_SYS_VFS_H=1 -DHAVE_SYS_MOUNT_H=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBSOCKET=1 -DFUNCPROTO=15 -DHAVE_XSHM=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STATVFS=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_NL_LANGINFO=1 -DHAVE_STRTOK_R=1 -DHAVE_IOS_BINARY=1 -DHAVE_CPP_EXPLICIT=1 -DHAVE_CPP_SPECIALIZATION=1 -DHAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX=1 -DHAVE_CPP_PARTIAL_SPECIALIZATION=1 -DHAVE_CPP_ACCESS_CHANGING_USING=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_NAMESPACE_STD=1 -DHAVE_CPP_UNAMBIGUOUS_STD_NOTEQUAL=1 -DHAVE_CPP_NEW_CASTS=1 -DHAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR=1 -DNEED_CPP_UNUSED_IMPLEMENTATIONS=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_DEFAULT_TOOLKIT=\"xlib\" -DMOZ_WIDGET_XLIB=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_X11=1 -DIBMBIDI=1 -DSUNCTL=1 -DACCESSIBILITY=1 -DMOZ_MATHML=1 -DMOZ_LOGGING=1 -DDETECT_WEBSHELL_LEAKS=1 -DMOZ_USER_DIR=\".mozilla\" -DMOZ_XUL=1 -DINCLUDE_XUL=1 -DNS_MT_SUPPORTED=1 -DUSE_IMG2=1 -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DJS_THREADSAFE=1 -DNS_PRINT_PREVIEW=1 -DMOZ_REFLOW_PERF=1 -DMOZ_REFLOW_PERF_DSP=1 ../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/xl ib/nsDeviceContextSpecXlib.cpp "../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/gfx/src/x lib/nsDeviceContextSpecXlib.cpp", line 559: Error: nsPrinterEnumeratorWin is not defined. 1 Error(s) detected. gmake[4]: *** [nsDeviceContextSpecXlib.o] Error 1 gmake[4]: Leaving directory `/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib/gfx/src/xlib' gmake[3]: *** [libs] Error 2 gmake[3]: Leaving directory `/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib/gfx/src' gmake[2]: *** [libs] Error 2 gmake[2]: Leaving directory `/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib/gfx' gmake[1]: *** [tier_9] Error 2 gmake[1]: Leaving directory `/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib' gmake: *** [default] Error 2 -- snip -- s/nsPrinterEnumeratorWin/nsPrinterEnumeratorXlib/ and it compiles ...
Per Marc's comments: Fixed static casts and put in the asserts (thought about it, should have done it) mDebugFile & mDebugFD point to the same file handle, there are two because it then enables us to ifdef out the some debug code, but I still need the file handle or it would have made ifdef'ing out the code much more complicated. These are for printing regression tests mDebugFilePtr - is for debugging printing. my minor change to nsRuleNode will not be checked in, I forgot to remove it from the patch. Not sure what to do with the result from calling prefs, pretty much nobody ever checks that.
Question/Issues: 1. I can go to the "Print preview" without selecting a printer first - which will cause confusion by the user when he/she later steps to the print dialog and sets an other printer there - which may end-up in a completely different page layout. 2. If no printer is found the print preview still shows a "preview". 3. Moving the page size selection to the "Page setup" dialog is not a good idea. Not all printers support all paper sizes (and they should not - admins may want to restrict the list of available paper sizes). IMHO paper size selection should to back into the print job options dialog there it was before (same applies to page layout). 4. Selecting a valid printer, printing with it and then steppting into print preview gives me a weired new error: (I placed an abort(); call into the X error handler of the Xprint module) -- snip -- ###!!! ASSERTION: frame was not removed from primary frame map beforedestruction or was readded to map after being removed: '!PL_DHASH_ENTRY_IS_BUSY(entry) || entry->frame != aFrame', file ../../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/layout/html/base/src/nsFrameManager.cpp, line 984 ###!!! Break: at file ../../../../../../../../../home/mozilla/src/2001-12-17-08-trunk/mozilla/layout/html/base/src/nsFrameManager.cpp, line 984 nsGfxXprintModule: Warning (X Error) - BadDrawable (invalid Pixmap or Window parameter) t@1 (l@1) signal ABRT (Abort) in __sigprocmask at 0xfee59d88 0xfee59d88: __sigprocmask+0x0008: jmp %o7 + 0x8 Current function is xerror_handler 80 abort(); (/opt/SUNWspro/bin/../WS6U2/bin/sparcv9/dbx) where current thread: t@1 [1] __sigprocmask(0x0, 0xffbee448, 0x0, 0xffffffff, 0xffffffff, 0x0), at 0xfee59d88 [2] _resetsig(0xfee6ca1c, 0x0, 0x0, 0x0, 0x37754, 0x37758), at 0xfee4f3c4 [3] _sigon(0xfee74330, 0xfee74310, 0x37750, 0xffbee51c, 0x6, 0xfed4e118), at 0xfee4eb80 [4] _thrp_kill(0x0, 0x1, 0x6, 0xfee6ca1c, 0x376d8, 0xfeac7b50), at 0xfee51954 [5] abort(0xfedb5d58, 0x59, 0xfedbd844, 0xfedb99a8, 0xfe99c000, 0x0), at 0xfed395b8 =>[6] xerror_handler(display = 0x9cbcf8, ev = 0xffbee6c8), line 80 in "nsXPrintContext.cpp" [7] _XError(0xfe9a1a6c, 0xffbee7f0, 0xfe99c000, 0x9cbcf8, 0x20, 0x9cbcf8), at 0xfe922884 [8] _XReply(0x9cbcf8, 0x114, 0x0, 0x114, 0x114, 0x0), at 0xfe914c6c [9] XGetGeometry(0x9cbcf8, 0xac00416, 0xffbee8b0, 0xffbee8c8, 0xffbee8c4, 0xffbee8c0), at 0xfe922654 [10] nsRenderingContextXlib::CommonInit(this = 0x94aca8), line 222 in "nsRenderingContextXlib.cpp" [11] nsRenderingContextXlib::Init(this = 0x94aca8, aContext = 0xb704d8, aWindow = 0xa4b4c0), line 189 in "nsRenderingContextXlib.cpp" [12] DeviceContextImpl::InitRenderingContext(this = 0x815108, aContext = 0x94aca8, aWin = 0xa4b4c0), line 240 in "nsDeviceContext.cpp" [13] DeviceContextImpl::CreateRenderingContext(this = 0x815108, aWidget = 0xa4b4c0, aContext = (nil)), line 224 in "nsDeviceContext.cpp" [14] nsDeviceContextXlib::CreateRenderingContext(this = 0x815108, aWidget = 0xa4b4c0, aContext = (nil)), line 58 in "nsDeviceContextXlib.h" dbx: warning: can't find file "/shared/bigtmp2/mozilla/2001-12-17-08-trunk/objdir_ws6_xlib/layout/build/nsPresShell.o" dbx: warning: see `help finding-files' [15] PresShell::CreateRenderingContext(0xfc856a9c, 0xfc82b8a8, 0xffbeec58, 0xffbeeb14, 0x9625c0, 0xfc391070), at 0xfc1d43d4 [16] PresShell::ResizeReflow(0x83d1d0, 0xfc5b83b0, 0xfc1d41d0, 0xfc8574c8, 0xfc83c6ec, 0xfc5d06d0), at 0xfc1d1afc [17] PresShell::ResizeReflow(0x83d1d0, 0x9625c0, 0x45fe, 0x25da, 0xfc82c0c4, 0x800), at 0xfc1da48c [18] nsViewManager::SetWindowDimensions(this = ???, width = ???, height = ???) (optimized), at 0xfc8362bc (line ~568) in "nsViewManager.cpp" [19] nsViewManager::DispatchEvent(this = ???, aEvent = ???, aStatus = ???) (optimized), at 0xfc838e80 (line ~1720) in "nsViewManager.cpp" [20] HandleEvent(aEvent = ???) (optimized), at 0xfc82b118 (line ~80) in "nsView.cpp" [21] nsWidget::DispatchEvent(this = ???, aEvent = ???, aStatus = ???) (optimized), at 0xfdb398d8 (line ~1237) in "nsWidget.cpp" [22] nsWidget::DispatchWindowEvent(this = ???, aEvent = STRUCT) (optimized), at 0xfdb39828 (line ~1135) in "nsWidget.cpp" [23] nsWidget::OnResize(this = ???, event = STRUCT) (optimized), at 0xfdb39800 (line ~1128) in "nsWidget.cpp" [24] nsAppShell::HandleConfigureNotifyEvent(event = ???, aWidget = ???) (optimized), at 0xfdb2b1ac (line ~794) in "nsAppShell.cpp" [25] nsAppShell::DispatchXEvent(event = ???) (optimized), at 0xfdb2a808 (line ~524) in "nsAppShell.cpp" [26] nsAppShell::Run(this = ???) (optimized), at 0xfdb2a1c0 (line ~339) in "nsAppShell.cpp" [27] nsAppShellService::Run(this = ???) (optimized), at 0xfdaab830 (line ~302) in "nsAppShellService.cpp" [28] main1(argc = ???, argv = ???, nativeApp = ???) (optimized), at 0x16ec0 (line ~1281) in "nsAppRunner.cpp" [29] main(argc = ???, argv = ???) (optimized), at 0x178a4 (line ~1606) in "nsAppRunner.cpp" -- snip -- or short: For some unknown reason we're calling |nsRenderingContextXlib::Init(nsIDeviceContext* aContext, nsIWidget *aWindow)| (where |aContext| seems to be a |nsDeviceContextXP| (Xprint device context) object) instead of |nsRenderingContextXp::Init(nsIDeviceContext* aContext, nsIWidget *aWindow)| (which would simply return a NS_ERROR_NOT_IMPLEMENTED).
Roland: As a tech support rep for Lexmark, I have to disagree with point four. Every application that has a page setup dialog does indeed give the user the ability to choose the page size in that dialog. Furthermore, the application slurps in the supported features from the printer driver (including page size), which allows the user to set the page size, as well as other printer dependent features, such as source tray. Margins are also printer dependent, since they depend on the size of the unprintable region. Good applications warn the user when the margins are set to print outside the unprintable region of the printer.
I am getting a crash in Xprint module with the following steps: 1. Print on a valid printer 2. Enable print preview (Menu File/Print preview) 3. Print again Result: Crash due lame error checking The attached patch fixes this issue in Xprint module - but other print modules should be checked for the same issue, too ...
It seems that when we print while print preview is active we're creating a 2nd printer nsDeviceContext instance... I doubt that this will ever work properly due many many global vars in the print modules... ;-(
More issues: 1. It seems that the print progress dialog comes up AFTER layout phase. It may be more usefull to create it before the layout phase that the user can see what's going on (may be usefull for printing large documents like http://sunsolve.sun.com/pub-cgi/show.pl?target=patchrpts/7&type=0 ...) 2. RFE: The print progress dialog should show the number of pages and the current page being printed in the dialog (what about the current status - like "initing"(=layout phase), "printing"(=printing pages) and "finishing"(=being in EndDocument() (which may take some seconds on some platforms))), too (what about updating the window title) ? 3. Switching to "print preview" does not turn on the busy pointer while relayouting the page (which takes a lot of time while processing the example URL above). 4. Trying to view http://sunsolve.sun.com/pub-cgi/show.pl?target=patchrpts/7&type=0 in page preview seems to consume an insane amount of time or hangs (I killed my debug build mozilla-bin after 15mins) ... 5. RFE: Print preview should open in a new browser TAB (what about making tis a prefs item ?)
Attachment #62245 - Attachment is obsolete: true
Please file separate Print Preview bug for these new issues when printing in PP. Also, the PrintProgress comes up before laying out the document. The problem is I don't think that thread ever gets a chance at the CPU. So I will have to look into that. PLus, if it takes a really long time (longer than laying out the page) then it is a layout problem when laying out for printing (file a bug) We talked about showing the number of pages but decided against it. becuase of all the ways documents could be printed.
Attached patch hopefully, the final patch (deleted) — Splinter Review
This includes the Xprint patch
I've finally gotten a chance to test the last patch posted for BeOS. nsPrintOptionsBeOS needs to be created in order for anything to work. I created it, and added it to nsGfxFactoryBeos. I was able to get the print dialog, and run print preview. The print priview to not draw properly, and an error box was shown when bringing up the page properties dialog. There may have been an error during the patching process that caused these problems, and I didn't catch it, or it's just that printing is not implemented under BeOS. I am going to try and re-apply the patch tomorrow, and then start looking into it more. Either way, the nsPrintOptionsBeOS needs to be created and added to nsGfxFactoryBeOS. I'll attach a patch when I get more information.
Attached patch nsDeviceContextSpecFactoryM.cpp patch (obsolete) (deleted) — Splinter Review
Attached patch nsDeviceContextSpecMac.cpp patch (obsolete) (deleted) — Splinter Review
Attached patch nsDeviceContextSpecMac.h patch (obsolete) (deleted) — Splinter Review
last of three patches attached to get this working on the mac. Had to do it this way because of Rods patch being in my tree.
With rods' changes, we should be able to move the Carbon session-based printing APIs; does the patch do that? Also, I've love to see nsDeviceContextSpecMac renamed to something that indicates that it's used only for printing (the 'Spec' part doesn't mean anything to me).
Zip file containing a beos implementation of nsPrintOptios
Patch to include the new nsPrintOptionsBeOS
With the above two patches, you can get the print preview to work under beos, everything looks fine. Some errors are thrown going into the page setup, but this is probably due to lack of printing support under beos. If it does take much to implement, I might have a patch for that coming soon as well, but that can be put under a different bug id. One question, however. Print preview, the only way to turn it off is to select print preview again. There seems to be no real indication that you are actually in print preview, other than the way the window looks. Could a check mark be placed next to the print preview menu option, when in print preview mode? For me, it's ok, but for users, well, that's another story. Anyway, if everyone else is ok with the new interface, with the above patches, I'm ok with this for BeOS. r=
Attachment #62939 - Attachment is obsolete: true
Attachment #62940 - Attachment is obsolete: true
Attachment #62942 - Attachment is obsolete: true
What happend to the printer properties on linux with the current build as of 01-01-2002? Allmost all (size, orientation, ...) are gone, is that related to the checkins from this bug?
Oops, there are two wrong UI texts in PrintProgress.dtd (already checked in at 12/14 but added to the build now) that are easy to fix but look very bad: http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/locale/en-US/printProgress.dtd shows 7 <!ENTITY keepProgressDialogUpMsg.label "Keep this window open after the message is successfully sent."> We do NOT send a message, we just are printing here! I propose "Keep this window open after priniting is finished." 20 <!ENTITY printComplete "Prnit is complete."> Typo: Could we please speak of "Print" instead of "Prnit"? ;-) I saw this in a trunk build of 2002-01-01.
Rod, the new Page Setup dialog looks great. I had to work to convince someone that it wasn't native ;-) However, I think replacing the Print dialog with a handrolled xul one was a mistake. Why was this done? The original bug reported was "Each platform needs infrastructure for being to "fly" a native dialog, or there own dialog", which seems correct to me. Windows has a print dialog that users expect, much as they have a filepicker that they expect. Why aren't we using it anymore?
Attached patch nsDeviceContextSpecOS2.cpp & .h (obsolete) (deleted) — Splinter Review
I built mozilla today on OS/2 and found that printers weren't being found when trying to print or use print preview. I modified nsDeviceContextSpecOS2.cpp/.h by using a GlobalPrinters class like gtk. This fix needs to be checked in for printing and print preview to work again on OS/2.
Any idea why this change would have slowed down page load on btek tinderbox by about 6ms? (That's not much, but I wouldn't have expected any change.) The ten btek times before the checkin were: 1235 1230 1230 1236 1231 1231 1230 1234 1237 1232 (mean=1232.6) and the ten btek times after the checkin were: 1239 1242 1237 1239 1240 1238 1236 1238 1240 1240 (mean=1238.9) with no other checkins. A t-test gives p=1.3e-5. Also, the following change: - nsAutoString propName; - propName.AssignWithConversion(aPropName); - PRUnichar* uPropName = ToNewUnicode(propName); + const PRUnichar* uPropName = NS_ConvertUTF8toUCS2(aPropName).get(); leads to the use of a dangling pointer. Why not just drop the null check and use NS_ConvertUTF8toUCS2 inline?
r=rods for the OS2 patch jessica, do you want me to check it in?
dbaron, Why not check for a null pointe? There is no reason to call GetLocalizedString string if the pref isn't there. And I am not sure how it leads to a dangling pointer. I that the const get was one of the reasons to use it? Also, I have gone thru my code with a fine tooth comb and can not find why it would slow down page load. All the code is for printing and I really can find where any of the code I changed actually gets run when a page loads (except for the minor change to nsLayoutModules). sleestack was going to back out my changes on btek and see what happens. But I am at a loss as to what is wrong.
> Why not check for a null pointe? It's never going to give a null pointer unless it's out of memory. It will return an empty string if you pass in null. It's a dangling pointer because the buffer it allocates is destroyed when the temporary goes away.
Attached patch Fixes memory leak (deleted) — Splinter Review
Comment on attachment 63517 [details] [diff] [review] Fixes memory leak r=dbaron, although it's not a leak, it's a free memory read.
Attachment #63517 - Flags: review+
Comment on attachment 63307 [details] [diff] [review] nsDeviceContextSpecOS2.cpp & .h Thanks Rod, but this patch got checked in under bug 117919. Marking obsolete.
Attachment #63307 - Attachment is obsolete: true
fixed
fixed, really
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
The "Printing Status" attachment holds out the hope that there will be an nsIPrintDialogService which will allow embedders to override printing UIs. Bug 61672 seems to cover this but it's been untouched since october. Is progress being tracked somewhere else? is there any progress at all?
Rod, can you verify this and mark verified-fixed? thanks.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: