10566 - Active X embeding component build configured improperly

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: ActiveX Wrapper, defect, P3)

Description

[tague]

The active X embeding component is always built when the MSSDK variable is
defined; other components are being added which require the SDK and need to be
built separately from the active X control.

Please update the active X control build to only build when an environment
variable (other than MSSDK) is defined.  See the attached patch.

Comment 1

[tague]

Attachment: makefile patch to not build active X when MSSDK is defined

Comment 2

[tague]

updated status to blocker and M9

Comment 3

[locka]

The patch is similar to something I suggested to Daniel Nunes and I heartily
support the idea. MSSDK is a dumb test here and anywhere else and should
something more obvious.

My idea was for a more general MOZ_ACTIVEX_SUPPORT environment variable. Not
only would allow the control to be built (or not) but also conditionally compile
other ActiveX bits and pieces in particular into Mozilla.

Comment 4

[tague]

Okay, so it sounds like we are in agreement.  Can you be sure to make this fix
for M9.  I'd like to start landing some other functionality which requires the
MSSDK to build.

Comment 5

[locka]

I don't have write access to mozilla/webshell/embed/makefile.win so I am unable
to make the change.

The required changes are these though:

RCS file: /cvsroot/mozilla/webshell/embed/makefile.win,v
retrieving revision 1.5
diff -r1.5 makefile.win
21c21
< !if ("$(MSSDK)" != "") || defined(NGPREFS)
---
> !if defined(MOZ_ACTIVEX_SUPPORT) || defined(NGPREFS)
23c23
< !if "$(MSSDK)" != ""
---
> !if defined(MOZ_ACTIVEX_SUPPORT)

Comment 6

[tague]

okay, i'll go in and fix them.  leaf do you have any problem with this?

Comment 7

[Daniel (Leaf) Nunes]

Looks good to me... though i'd prefer to have negative logic here... and have
the activex control get built by default.

Comment 8

[tague]

leaf :- okay, i'll make the test negative.

it would be nice to build the activeX control by default, but that requires
having the MSSDK installed on your development machine which some people may be
oppposed to.  i work on pushing the transition if you want, because i'm going to
need it for other features.

Comment 9

[chris hofmann]

looks like this should be assigned tague..

Comment 10

[locka]

Seems fine to me. Test for something like MOZ_ACTIVEX_NO_SUPPORT to determine
if it should be built or not.

Comment 11

[tague]

I checked this fix in today.  the emebding directory no longer looks at the
MSSDK variable.  I also changed the switch for ActiveX embeding component to
MOZ_ACTIVEX_SUPPORT as requested.

/t

Comment 12

[cpratt]

As the reporter checked in the fix, and as I am not capable of verifying the
fix, I'm going to take the reporter's word for it and mark this verified fixed.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fcd24391229b
[fenix] for https://github.com/mozilla-mobile/fenix/issues/9407: fixed and added collection tests (https://github.com/mozilla-mobile/fenix/pull/10566)