719389 - Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Serge Gautherie (:sgautherie)]

Noticed when trying to build using "--with-windows-version=502".

https://tbpl.mozilla.org/?tree=Try&rev=b6ec764aef44
https://tbpl.mozilla.org/php/getParsedLog.php?id=8663207&tree=Try
WINNT 5.2 try build on 2012-01-19 02:34:26 PST for push b6ec764aef44
{
nsAppShell.cpp

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(258) : warning C4067: unexpected tokens following preprocessor directive - expected a newline

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C3083: 'widget': the symbol to the left of a '::' must be a type

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C2039: 'StopAudioSession' : is not a member of 'mozilla'

e:/builds/moz2_slave/try-w32/build/widget/src/windows/nsAppShell.cpp(259) : error C3861: 'StopAudioSession': identifier not found

...

make[7]: *** [nsAppShell.obj] Error 2
}

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197 [Checked in: Comments 3 and 11]

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

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

Heh, I feel dumb.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

https://hg.mozilla.org/mozilla-central/rev/4e12e9a68795


[Approval Request Comment]
Regression caused by (bug #): bug 441197.
User impact if declined: not sure, but blocks building using "--with-windows-version=502".
Testing completed (on m-c, etc.): multiple Try builds and this comment.
Risk to taking this patch (and alternatives if risky): no risk, trivial typo fix.

Comment 4

[Alex Keybl [:akeybl]]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

[Triage Comment]
It's not clear what user impact not being able to build with "--with-windows-version=502" has, and thus does not meet the requirement for Aurora/Beta uplift. Please re-nominate once that's known.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

(In reply to Alex Keybl [:akeybl] from comment #4)
> It's not clear what user impact not being able to build with
> "--with-windows-version=502" has, and thus does not meet the requirement for
> Aurora/Beta uplift. Please re-nominate once that's known.

"--with-windows-version=502" means:
https://developer.mozilla.org/En/Windows_SDK_versions#section_9
"Windows Server 2003 R2 Platform SDK
 This is your only option if you're on Windows 2000."

This impossibility to build is not part of official builds, but can affect local/customized builds.

Also I didn't check how official builds actually handle this syntax error: I assume they happen to work (around it), but better be correct/sure/safe.

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Serge Gautherie (:sgautherie) from comment #5)
> This impossibility to build is not part of official builds, but can affect
> local/customized builds.
> 
> Also I didn't check how official builds actually handle this syntax error: I
> assume they happen to work (around it), but better be correct/sure/safe.

Presumably a user can apply this patch to their local build in that case. We'll approve for Aurora, but it's too late in the game for FF10.

Comment 7

[Serge Gautherie (:sgautherie)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=8780739&tree=Firefox
WINNT 5.2 mozilla-central build on 2012-01-23 22:08:54 PST for push 3be494e4cc8f
still succeeds.

V.Fixed

*****

(In reply to Alex Keybl [:akeybl] from comment #6)

> Presumably a user can apply this patch to their local build in that case.

Sure.

> We'll approve for Aurora, but it's too late in the game for FF10.

Sad, as iiuc FF10 is meant to be LTS.

Comment 8

[Ed Morley [:emorley]]

Missed aurora; removing checkin-needed to tidy checkin-needed saved search since presume you need to re-request for beta (if still wanted) anyway.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

Per comment 8.

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

[Triage Comment]
This patch has now baked on m-c (and Aurora 12 since we pushed it out Friday). Approving for Beta 11.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 590205 [details] [diff] [review]
(Av1) Fix "#ifdef MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN" from bug 441197
[Checked in: Comments 3 and 11]

http://hg.mozilla.org/releases/mozilla-beta/rev/004bfb5325af

Comment 12

[Serge Gautherie (:sgautherie)]

https://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=afcb5dfe30eb
is still green.

firefox11: verified.