In addition to the patch, all files in gfx/src/os2/ will be cvs removed except the following: nsGfxDefs.h nsOS2Uni.cpp nsOS2Uni.h nsPaletteOS2.cpp nsPaletteOS2.h nsPrintOS2.cpp nsPrintOS2.h which are still built for cairo-os2 builds: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/Makefile.in&rev=1.159&root=/cvsroot&mark=114-125#114 (I have no clue whether the code is actually still used though)

Mats, thanks for your effort. I would prefer to leave the code in the tree for the moment until we have solved some basic problems of cairo-os2 builds like plugins and printing. It's just easier to have something in the tree to look at instead of having to keep another tree around... But I should take a look if the Makefile lines you point out are still necessary.

Yes, those files are indeed still used. They could be moved to widget with a little effort. With a few more changed lines nsPaletteOS2.* and then also nsGfxDefs.h can be made obsolete. I guess that palette management will not work with cairo surfaces any more anyway. I am not sure if a cairo-os2 build will even run on a machine with 256 colors where palette management would be needed. The patch looks fine to me but as I said, let's postpone the actual change a bit.

Note, the non-cairo stuff is the OS/2 code blocks cleanup of gfxImageFrame.

Comment on attachment 274048 [details] [diff] [review] Patch rev. 1 OK, the obsolete code is starting to annoy me, too, while working on other stuff in nsWindow. So let's get this in soonish. >Index: widget/src/os2/nsWindow.cpp [...] > #include "nsGUIEvent.h" > #include "nsIRenderingContext.h" >-#ifdef MOZ_CAIRO_GFX > #include "gfxOS2Surface.h" > #include "gfxContext.h" >-#else >-#include "nsIRenderingContextOS2.h" >-#endif [...] >Index: widget/src/os2/nsWindow.h [...] >- >-#ifdef MOZ_CAIRO_GFX > #include <gfxOS2Surface.h> >-#endif While doing the above changes to nsWindow, can you move both lines #include "gfxOS2Surface.h" #include "gfxContext.h" from nsWindow.cpp to nsWindow.h instead of #include <gfxOS2Surface.h>? About that files listed in comment 0, let's copy them over to widget/src/os2 and add them to CPPSRCS in the Makefile there for the moment. Then deal with cleaning them up in a separate bug. (I think at this points it's difficult to completely remove all of them.)

Comment on attachment 274048 [details] [diff] [review] Patch rev. 1 Not sure if approval is required since this is OS/2 only, but asking anyway...

Mats, no OS/2 doesn't need approval. To quote mconnor "For anything OS/2 only I think we'd have a blanket approval at this point." (from mozilla.dev.planning). But before checking this is we should handle the files from comment 1 somehow. I had forgotten about that but now I filed bug 394857 about them.

Ok, I addressed your comments and I was just about to land it when I saw bug 394857 was reopened. I'll hold off until that bug is resolved.

Checked in to trunk at 2007-09-08 09:22 PDT. -> FIXED