Comment on attachment 357072 [details] [diff] [review] patch v.1 r=me for the remaining nsScreenWin.cpp part.

Doug, I'm happy to unbitrot this, but before I do so: 1) Is this still wanted? ie: Your reply in bug 614720 comment 2 implies hesitance to strip out WinCE stuff. However, Mike's comment in bug 647389 comment 4 seems to imply that removing WinCE is acceptable, given that a lot of things would need to change if WinCE was supported in the future anyway. 2) There is a lot more other widget WINCE stuff that can be stripped out, per: http://mxr.mozilla.org/mozilla-central/search?string=wince&find=widget ...would you want me to do that too?

(In reply to comment #2) > 1) Is this still wanted? Answering my own question: WinCE/Windows Mobile support has already been removed from the main build system, Spidermonkey, mobile installer, in-app updater and so on (see bug 614720, bug 554087 and all their dependants); so building isn't possible on WinCE anyway. Until such point where MS decide to release a Windows Phone 7 NDK and the decision is made to port to that platform, the WinCE/WinMO code is unnecessary cruft that is only going to complicate code maintenance, so IMHO should be removed. In X years time if a port goes ahead, and if any of the old WinCE code is deemed still useful for reference; then that's what a VCS is for. > 2) There is a lot more other widget WINCE stuff that can be stripped out, per: > http://mxr.mozilla.org/mozilla-central/search?string=wince&find=widget Since the bug title is about widget code in general; going to just cover it all here: http://mxr.mozilla.org/mozilla-central/search?string=wince&find=%2Fwidget%2F&tree=mozilla-central http://mxr.mozilla.org/mozilla-central/search?string=windows+ce&find=%2Fwidget%2F&tree=mozilla-central http://mxr.mozilla.org/mozilla-central/search?string=windows+mob&find=%2Fwidget%2F&tree=mozilla-central

Files to be removed: http://mxr.mozilla.org/mozilla-central/find?string=%2Fwidget%2Fsrc%2Fwindows%2Fns(Window|Clipboard)CE\..*&tree=mozilla-central Consumers: http://mxr.mozilla.org/mozilla-central/search?string=%28window|clipboard%29ce&regexp=1&filter=^[^\0]*%24&tree=mozilla-central

Almost done with the patch, but quick question: Any idea whether SND_PURGE is still required? http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound.cpp#118 > #ifndef SND_PURGE > // Not available on Windows CE, and according to MSDN > // doesn't do anything on recent windows either. > #define SND_PURGE 0 > #endif It is only ever set to 0 and is only used here: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound.cpp#144 and http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound.cpp#229 Also, the MSDN doc here says it's not supported: http://msdn.microsoft.com/en-us/library/dd743680%28VS.85%29.aspx

Hg rm: nsWindowCE.(h|cpp) nsClipboardCE.(h|cpp)

Removes all WinCE & Windows Mobile ifdef'ed code from widget/* nsWindowGfx::OnSettingsChangeGfx is now empty as it previously only contained |ifdef WINCE_WINDOWS_MOBILE| reachable code. Will remove the placeholder comment separately (in part D) to keep things easier to follow.

After part B is applied, ENABLE_IME_MOUSE_HANDLING is always defined as 1: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsIMM32Handler.h#61 This patch removes the now superfluous ENABLE_IME_MOUSE_HANDLING define/ifdefs: http://mxr.mozilla.org/mozilla-central/search?string=ENABLE_IME_MOUSE_HANDLING

After part B is applied, nsWindowGfx::OnSettingsChangeGfx is now just an empty stub: https://bugzilla.mozilla.org/attachment.cgi?id=527169&action=diff#a/widget/src/windows/nsWindowGfx.cpp_sec2 Its only caller is nsWindow::OnSettingsChange, which once the redundant call to nsWindowGfx::OnSettingsChangeGfx is removed, also now doesn't do anything: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#7668 Therefore this patch removes... - nsWindowGfx::OnSettingsChangeGfx - nsWindow::OnSettingsChange - The only call to nsWindow::OnSettingsChange: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#5501

Note: Parts C & D require B to have been applied first, or they won't apply cleanly.

Comment on attachment 527164 [details] [diff] [review] Part A - File removal rs+

Comment on attachment 527169 [details] [diff] [review] Part B - Main ifdef removal rs+

please push to try before pushing to m-c (or use a project branch)

and thanks!

(In reply to comment #13) > please push to try before pushing to m-c (or use a project branch) Cool will do (once bug 648541 happens). Thanks for the review :-)

Pushed to try yesterday: http://dev.philringnalda.com/tbpl/?tree=Try&rev=84e893e5949a However for some reason hg decided that a load of changesets already in m-c needed to be included; was this just caused by the try repo not being up to date with m-c tip due to try being closed for part of yesterday? Or did I do something wrong? Also, there were 3 oranges in the results, but from what I can tell they are intermittent failures. I've used the rebuild command in the self serve API to trigger those tests again; presuming this is what I'm supposed to do in those situations? After rerunning the tests, all but one now passes - and that one has an associated bug that is perma-orange. However the bug only mentions one of the many errors for that test - ideas? Sorry for all the questions; first time using try and the try server wiki page doesn't cover much beyond how to submit tests and where to look for results. Thanks!

Also, any idea about comment 5?

Only change: Updated to tip & r=dougt added to commit message.

Only change: Updated to tip & r=dougt added to commit message.

Only change: Updated to tip & r=dougt added to commit message.

Only change: Updated to tip & r=dougt added to commit message.

All four parts ready for checkin (passed try http://dev.philringnalda.com/tbpl/?tree=Try&rev=84e893e5949a and queries I had in comment 16 have been answered on IRC). r+ from dougt has been carried forwards. Note: They need to be applied in order A->D, or there will be merge conflicts. Thanks!

http://hg.mozilla.org/mozilla-central/rev/c4864738e3fd http://hg.mozilla.org/mozilla-central/rev/2460d5f02a5d http://hg.mozilla.org/mozilla-central/rev/d33506667f79 http://hg.mozilla.org/mozilla-central/rev/d29e9cb9d0c9

Thanks Zack, much obliged :-)

Doug, any ideas about this? Thanks :-) (In reply to comment #5) > Any idea whether SND_PURGE is still required? > > http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound. > cpp#118 > > #ifndef SND_PURGE > > // Not available on Windows CE, and according to MSDN > > // doesn't do anything on recent windows either. > > #define SND_PURGE 0 > > #endif > > It is only ever set to 0 and is only used here: > http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound. > cpp#144 > and > http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsSound. > cpp#229 > > Also, the MSDN doc here says it's not supported: > http://msdn.microsoft.com/en-us/library/dd743680%28VS.85%29.aspx