Get printing working in Qt toolkit

Taking myself...

Comment on attachment 109474 [details] [diff] [review] Patch to get both Xprint and PostScript modules working again (no QPrinter support yet) the patch does not apply cleanly to current trunk :(

Comment on attachment 109474 [details] [diff] [review] Patch to get both Xprint and PostScript modules working again (no QPrinter support yet) It applies to my trunk build cleanly... ;-/

note, timeless(@bemail.org) should review such patches, as he is the owner of the qt port...

Comment on attachment 109474 [details] [diff] [review] Patch to get both Xprint and PostScript modules working again (no QPrinter support yet) +NS_IMETHODIMP nsDeviceContextSpecFactoryQT::CreateDeviceContextSpec(nsIWidget *aWidget, + nsIPrintSettings* aPrintSettings, + nsIDeviceContextSpec *&aNewSpec, + PRBool aIsPrintPreview) { + nsresult rv; + static NS_DEFINE_CID(kDeviceContextSpecCID, NS_DEVICE_CONTEXT_SPEC_CID); + nsCOMPtr<nsIDeviceContextSpec> devSpec = do_CreateInstance(kDeviceContextSpecCID, &rv); i think this can be an early return: + if (NS_SUCCEEDED(rv)) -- if there's some reason not to early return, please explain :) + { use NS_..._CAST: (you've used it elsewhere) + rv = ((nsDeviceContextSpecQT *)devSpec.get())->Init(aPrintSettings); - * Portions created by the Initial Developer are Copyright (C) 1999 + * Portions created by the Initial Developer are Copyright (C) 1998 this is just strange, please explain. ah, well -#ifndef nsDeviceContextSpecFactoryQT_h___ -#define nsDeviceContextSpecFactoryQT_h___ +#ifndef nsDeviceContextSpecFactoryX_h___ +#define nsDeviceContextSpecFactoryX_h___ please don't change the guard, it's confusing and wrong. personally i don't see a reason to remove all of the debugging bits that we've grown, we can remove them later. -#include "nsIPrefBranch.h" -#include "nsIPrefService.h" +#include "nsIPref.h" caillon will kill you. we're moving from nsIPref to nsIPrefBranch, don't undo his work. if you need help with nsIPrefBranch you can contact caillon on irc. note that i'd really like for the pref code you write to ctually use prefbranches instead of just relying on the root branch, it will make the code much cleaner. +/* Ensure that the result is always equal to either PR_TRUE or PR_FALSE */ +#define MAKE_PR_BOOL(val) ((val)?(PR_TRUE):(PR_FALSE)) this is silly, and since i'm the module owner i can refuse to accept it. you can use (PRBool)foo if you want to explicitly indicate that you're treating foo as a boolean, and you should use if (foo) or if (!foo) to test the PRBool. Actually, it doesn't even appear to be used... +// then it will never be delete, so this class takes care of that. delete=>deleted instead of using NS_IMPL_ISUPPORTSx w/ lots of #ifdef, look into using the INTERFACEMAP. +NS_IMETHODIMP nsPrinterEnumeratorQT::EnumeratePrinters(PRUint32* aCount, PRUnichar*** aResult) + NS_ENSURE_ARG(aCount); + NS_ENSURE_ARG_POINTER(aResult); + + if (aCount) + *aCount = 0; + else + return NS_ERROR_NULL_POINTER; change that first line to NS_ENSURE_ARG_POINTER and remove the else+return (which weren't reachable anyway). remove the else+return for aResult too. + nsresult rv = GlobalPrinters::GetInstance()->InitializeGlobalPrinters(); ... + PRInt32 numPrinters = GlobalPrinters::GetInstance()->GetNumPrinters(); wouldn't it make some sense to cache GetInstance() function calls aren't free. + NS_ENSURE_ARG_POINTER(aPrinterName); + NS_ENSURE_ARG_POINTER(aPrintSettings); + + NS_ENSURE_TRUE(*aPrinterName, NS_ERROR_FAILURE); + NS_ENSURE_TRUE(aPrintSettings, NS_ERROR_FAILURE); the second check of aPrintSettings is redundant (remove it) + const char *path; + + if (!(path = PR_GetEnv("PWD"))) + path = PR_GetEnv("HOME"); PR_GetEnv is actually expensive on some platforms, please make path a static and check to see if you've already picked one. + return NS_OK; + } + else +#endif /* USE_XPRINT */ you don't need the else. + PRBool allocate = (GlobalPrinters::GetInstance()->PrintersAreAllocated() == PR_FALSE); do this instead: + PRBool allocate = !(GlobalPrinters::GetInstance()->PrintersAreAllocated()); I don't see particularly good reasons to use int instead of PRInt_. timeless@boffo:~$ patch -p0 --dry-run <xpr patching file mozilla/gfx/src/qt/nsDeviceContextQT.cpp patching file mozilla/gfx/src/qt/nsDeviceContextSpecFactoryQT.cpp Hunk #3 FAILED at 39. 1 out of 3 hunks FAILED -- saving rejects to file mozilla/gfx/src/qt/nsDeviceContextSpecFactoryQT.cpp.rej patching file mozilla/gfx/src/qt/nsDeviceContextSpecFactoryQT.h patching file mozilla/gfx/src/qt/nsDeviceContextSpecQT.cpp Hunk #3 FAILED at 36. Hunk #4 succeeded at 324 (offset 4 lines). 1 out of 4 hunks FAILED -- saving rejects to file mozilla/gfx/src/qt/nsDeviceContextSpecQT.cpp.rej patching file mozilla/gfx/src/qt/nsDeviceContextSpecQT.h

btw, I saw exactly the same chunks failing when I tried to apply the patch. Roland, I guess you really have used an old version, which is missing the pref changes as well :(

Note that this patch is a 1:1 copy from the matching files from gfx/src/xlib/. I am just copying the files and doing a s/Xlib/QT/ here. Any further change must be ported to the other Unix toolkits to keep them in sync (which will require r=/sr=/a=/moa=/dancing_dog=/goat= and whatever evil is out there, too).

Comment on attachment 109532 [details] [diff] [review] Patch for 2002-12-16-08-trunk timeless: Can we get the current patch "in" to get Qt printing glue in gfx/ _working_ and then cleanup it after we migrated all copies in gfx/src/gtk, gfx/src/xlib and gfx/src/qt into gfx/src/unixshared (otherwise we would have to maintain three copies of the same code), please ?

Comment on attachment 109532 [details] [diff] [review] Patch for 2002-12-16-08-trunk ... waiting for something better doesn't help anyone

checked in .. please hurry up and unify this stuff

Marking bug as FIXED - this should be working now (code does not differ in functionality from other platforms, we still have to verify this once menus are working again...).