As the subject says - some new code for the Xprint module, minor fixes/changes/updates based on user/developer feedback... I'll file the patch in a few mins...

Swapping QA<-->Owner, setting milestone to 0.9.3

Filed patch, requesting r= for that beast... :-)

Filed new patch, requesting r= ...

fix the issues discussed in IRC #mozilla and you have r=zuperdee

Filed new patch to match r=zuperdee, requesting sr= ... tor/blizzard ?

The _IMPL_NS_XPRINT ifdefs all over this code make this an almost unmaintainable mess. I'd rather not see them go in there if possible.

_IMPL_NS_XPRINT is mandatory for StaticBuild stuff (and I used it in one place for something else... ;-( ). Currently there is no way around this - inheritance does not work, too (again, StaticBuild stuff will break). I am currently looking for a better solution (like making a common base class for some Xlib/Xprint classes to get rid of some CPP stuff), but currently this is the only way which works in all cases...

Filed bug 90380 ("Get rid of |#ifdef _IMPL_NS_XPRINT|") for this issue. Requesting sr= for this patch, please. There is a lot of work ToDo until 0.9.3 - and I want both working Xlib-toolkit and Xprint module in 0.9.3, please...

I don't care about whether or not the ifdef is in the code. I just want you to handle it better. For example, you have them scattered in the middle of functions with #else clauses as well. If there's a chance to do so please make two copies of the function, one inside the ifdef and one outside of it. When you have to either maintain the code or delete the ifdefs it makes things a lot easier. There are places where it is apporiate to have it in the middle of a function. For example in your initializer where you initialize variables. But when you start having to include #else clauses in the middle of functions that affect flow of that function it's not appropriate. Please clean up the code before you check it in.

Filed new patch to fix the issue, requesting sr= for that new patch...

I don't really like this revised patch either, as it still has lots of "#if[n]def _IMPL_NS_XPRINT" floating around the xlib code. In some ways they are worse now as you now duplicate code to get function level conditioning.

tor: That's why I filed bug 90380 - to get rid of most those |#ifdef|. But that isn't a thing I can roll-out in one day - I have more serious things in my 0.9.3-ToDo list for Xlib-toolkit stuff. I migrated parts of both modules to get faster development for both sides...

OK, I'll try to explain why there are some |#ifdef _IMPL_NS_XPRINT|: 1. The major difference between Mozilla print code and display code is that print code does not have an offscreen rendering surface, e.g. print code does not have or use nsDrawingSurface stuff. 2. Xprint code uses |nsXPrintContext| instead of |nsDrawingSurfaceXlib| 3. IMG2 requires some extra methods for printing images (see |#ifdef USE_IMG2|) which are only required for print code - not for display code. All these |#ifdef _IMPL_NS_XPRINT| except [1] can be removed in a future step - that's why I filed bug 90380...

I don't see any further ways to enhance the patch now (please correct me if I am wrong...). Requesting sr=, _please_...

CC:'ing mkaply@us.ibm.com for checkin once the sr= comes in...

As blizzard mentioned in his comment, sometimes inline #ifdefs are acceptable. In particular the constructor was named as an example, yet you duplicated that code. Why? Fix the constructor and a reluctant sr=tor.

> As blizzard mentioned in his comment, sometimes inline #ifdefs are acceptable. > In particular the constructor was named as an example, yet you duplicated > that code. Why? Er... unknown. I assume some sort of wild braindead hacking while in "panic" to make the superreviewer happy... ;-/ Sorry... ;-( New patch will follow in a few hours...

Filed final patch. Changes: - moved |#ifdef|-stuff into nsRenderingContextXlib constructor - removed use of |#ifdef USE_IMG2| from patch. This is an obsolete CPP symbol (imglib is dead) per pavlov's comment in IRC #mozilla. mkaply ?

Im going to have to do this tomorrow, unless someone can pick it up. I can't watch the tree tonight

Checked in