1325503 - Remove Windows {XP, Server 2003, Vista} support from widget/.

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Nicholas Nethercote [inactive]]

I like deleting code so I took a crack at removing widget support for the no-longer-supported versions of Windows. Patch coming up shortly.

Comment 1

[Masatoshi Kimura [:emk]]

Sorry if bug 1325368 is a duplicate effort.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Remove Windows {XP, Server 2003, Vista} support from widget/

I did my best to remove as much stuff as possible in this patch. The starting
point was to remove all the IsVistaOrLater() and IsWin7OrLater() calls, but I
also grepped for various strings and found some other removable stuff that way.
I may have still missed some things.

Notable things done by this patch.

- It removes numerous blocklist entries.

- It removes CanComputeVirtualKeyCodeFromScanCode(), because it's always true
  now.

- It removes ShowXP{Folder,File}Picker(), even though these were available as
  fallbacks on Vista+. The "when platform is built without the longhorn SDK"
  condition in the comment above nsFilePicker::ShowXPFolderPicker() sounds like
  it won't ever happen any more.

- It removes the config.trim_on_minimize preference. This requires adding a
  bool sHaveInitializedPrefs variable; previously the lack of pref
  initialization was indicated by the tri-state sTrimOnMinimize variable having
  the value 2.

Notable things *not* done by this patch.

- ClearThemeRegion() still exists. The comment suggests that it is XP/Vista
  only, but the code suggests otherwise. jimm thinks the comment is wrong.

- The comment in WinWakeLockListener::Callback() suggests that the StartTimer()
  call is no longer needed to block the screen saver. I'm uncertain about this
  and so I think it's best left to a follow-up.

Comment 3

[Nicholas Nethercote [inactive]]

emk did a similar change in bug 1325368. I think my changes are more comprehensive, e.g. I have changes in GfxInfo.cpp that emk's patch lacks. I also think it's better to break these changes up by component rather than doing them tree-wide. jimm, since you are already r?'d in bug 1325368 I'll let you decide the best way to proceed.

Comment 4

[Nicholas Nethercote [inactive]]

Try server looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2110991e94039a2c9d37da0d7189b4e38a9d2fc8

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Remove Windows {XP, Server 2003, Vista} support from widget/

jimm: you r-'d the full-repo patch in bug 1325368 for being too broad so I
think it makes sense to do the widget changes here. I have rebased the patch.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8824819 [details] [diff] [review]
Remove Windows {XP, Server 2003, Vista} support from widget/

Review of attachment 8824819 [details] [diff] [review]:
-----------------------------------------------------------------

r+, thanks for doing this. (Those two theme tests might cause this to bounce though.)

::: layout/style/nsMediaFeatures.cpp
@@ -71,5 @@
>  
>  // Os version identities used in the -moz-os-version media query.
>  const OperatingSystemVersionInfo osVersionStrings[] = {
> -  { LookAndFeel::eOperatingSystemVersion_WindowsXP,     L"windows-xp" },
> -  { LookAndFeel::eOperatingSystemVersion_WindowsVista,  L"windows-vista" },

We have css and some tests that will break when this gets removed.

http://searchfox.org/mozilla-central/search?q=windows-xp&path=

::: widget/windows/nsFilePicker.cpp
@@ -681,5 @@
> -  return GetFileNameWrapper(ofn, aType);
> -}
> -
> -bool
> -nsFilePicker::ShowXPFilePicker(const nsString& aInitialDir)

buh bye ancient filepicker code. woot!

::: widget/windows/nsNativeThemeWin.cpp
@@ -418,5 @@
>  // The amount of time we animate progress meters parts across the frame.
>  static const double kProgressDeterminateTimeSpan = 3.0;
>  static const double kProgressIndeterminateTimeSpan = 5.0;
>  // The width of the overlay used to animate the horizontal progress bar (Vista and later).
>  static const int32_t kProgressHorizontalVistaOverlaySize = 120;

you could name this 'kProgressHorizontalOverlaySize' if you like.

@@ -554,5 @@
>  }
>  
>  /*
> - * DrawChunkProgressMeter - renders an xp style chunked progress meter. Called
> - * by DrawProgressMeter.

ahh. eventually everything we write gets rewritten or removed.

@@ +1192,4 @@
>            return NS_OK;
>          }
> +      } else {
> +        /* On Vista, the dropdown indicator on a menulist button in chrome is

nit - snip "On Vista, "

@@ +2198,5 @@
>      case NS_THEME_SCALETHUMB_HORIZONTAL:
>      case NS_THEME_SCALETHUMB_VERTICAL:
>      {
>        *aIsOverridable = false;
> +      // On Vista, GetThemePartAndState returns odd values for

ditto here - nix "On Vista, "

::: widget/windows/nsWindow.cpp
@@ -845,5 @@
> -    // the working set when windows are minimized, but on Vista and up it has
> -    // little to no effect. Since this feature has been the source of numerous
> -    // bugs over the years, disable it (sTrimOnMinimize=1) on Vista and up.
> -    sTrimOnMinimize =
> -      Preferences::GetBool("config.trim_on_minimize",

'config.trim_on_minimize' - R.I.P.

Comment 8

[Nicholas Nethercote [inactive]]

> ::: layout/style/nsMediaFeatures.cpp
> @@ -71,5 @@
> >  
> >  // Os version identities used in the -moz-os-version media query.
> >  const OperatingSystemVersionInfo osVersionStrings[] = {
> > -  { LookAndFeel::eOperatingSystemVersion_WindowsXP,     L"windows-xp" },
> > -  { LookAndFeel::eOperatingSystemVersion_WindowsVista,  L"windows-vista" },
> 
> We have css and some tests that will break when this gets removed.

My try push (comment 4) didn't have any failures. However, this bug is supposed to be about widget/ and this change is in layout/ so I will revert. I'll file a follow-up for removal of these CSS properties.

Comment 9

[Nicholas Nethercote [inactive]]

> I'll file a follow-up for removal of these CSS properties.

Bug 1330146.

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0673be23886e7eadfa5d815394357db365b86f70
Bug 1325503 - Remove Windows {XP, Server 2003, Vista} support from widget/. r=jimm.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0673be23886e

Comment 12

[Masatoshi Kimura [:emk]]

Bug 751729 regressed because this bug completely removed the old file picker code. Is it intentional?

Comment 13

[Nicholas Nethercote [inactive]]

(In reply to Masatoshi Kimura [:emk] from comment #12)
> Bug 751729 regressed because this bug completely removed the old file picker
> code. Is it intentional?

From comment 2:

> - It removes ShowXP{Folder,File}Picker(), even though these were available as
>   fallbacks on Vista+. The "when platform is built without the longhorn SDK"
>   condition in the comment above nsFilePicker::ShowXPFolderPicker() sounds like
>   it won't ever happen any more.

jimm and I discussed this on IRC and he said it was ok. But I can reinstate that code if necessary.

Comment 14

[Masatoshi Kimura [:emk]]

(In reply to Nicholas Nethercote [:njn] from comment #13)
> > - It removes ShowXP{Folder,File}Picker(), even though these were available as
> >   fallbacks on Vista+. The "when platform is built without the longhorn SDK"
> >   condition in the comment above nsFilePicker::ShowXPFolderPicker() sounds like
> >   it won't ever happen any more.

The comment was added in bug 661991 and bug 751729 didn't update the comment accordingly.

> jimm and I discussed this on IRC and he said it was ok. But I can reinstate
> that code if necessary.

Please do.

Comment 15

[Jim Mathies [:jimm]]

(In reply to Masatoshi Kimura [:emk] from comment #14)
> (In reply to Nicholas Nethercote [:njn] from comment #13)
> > > - It removes ShowXP{Folder,File}Picker(), even though these were available as
> > >   fallbacks on Vista+. The "when platform is built without the longhorn SDK"
> > >   condition in the comment above nsFilePicker::ShowXPFolderPicker() sounds like
> > >   it won't ever happen any more.
> 
> The comment was added in bug 661991 and bug 751729 didn't update the comment
> accordingly.
> 
> > jimm and I discussed this on IRC and he said it was ok. But I can reinstate
> > that code if necessary.
> 
> Please do.

Wait, why are we bringing this back? For XP compatibility mode?

Comment 16

[Masatoshi Kimura [:emk]]

(In reply to Jim Mathies [:jimm] from comment #15)
> Wait, why are we bringing this back? For XP compatibility mode?

Written in bug 751729. If "Disable Visual Themes" is checked in Comptibility tab of the Properties dialog of firefox.exe, the file picker will no longer open. It does not have to do with XP compatibility mode.

Do we no longer support this configuration?

Comment 17

[Jim Mathies [:jimm]]

(In reply to Masatoshi Kimura [:emk] from comment #16)
> (In reply to Jim Mathies [:jimm] from comment #15)
> > Wait, why are we bringing this back? For XP compatibility mode?
> 
> Written in bug 751729. If "Disable Visual Themes" is checked in Comptibility
> tab of the Properties dialog of firefox.exe, the file picker will no longer
> open. It does not have to do with XP compatibility mode.
> 
> Do we no longer support this configuration?

I guess this is a decision we need to make. I don't know off hand how popular this option is but I'm betting it's not common at all. Also, there are additional issues with this option in the browser, including a problem with the titlebar command buttons not getting displayed.

Why don't we file a bug on this and see how many users chime in? There's an easy work around (uncheck the option).

FYI, I also tried XPSP2 compat mode and surprisingly, the browser launched and loaded a page.

Comment 18

[Nicholas Nethercote [inactive]]

I filed bug 1331278 about removing support for "Disable Visual Themes".