Closed
Bug 203362
Opened 22 years ago
Closed 22 years ago
Turn on building IDispatch code by default
Categories
(Core :: XPConnect, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: dbradley, Assigned: netscape)
References
Details
Attachments
(4 files, 3 obsolete files)
1.31 KB,
patch
|
dbradley
:
review+
alecf
:
superreview+
asa
:
approval1.4b+
|
Details | Diff | Splinter Review |
2.22 KB,
patch
|
bryner
:
review+
alecf
:
superreview+
sspitzer
:
approval1.4b+
|
Details | Diff | Splinter Review |
626 bytes,
patch
|
Details | Diff | Splinter Review | |
5.11 KB,
patch
|
netscape
:
review-
|
Details | Diff | Splinter Review |
This only turn on the building of the code. It will not actually provide
IDispatch support to Mozilla builds.
Reporter | ||
Comment 1•22 years ago
|
||
We need to get this into 1.4
Target Milestone: --- → mozilla1.4beta
Assignee | ||
Comment 2•22 years ago
|
||
Strictly speaking, that's not true, right? IDispatch will be built into
xpconnect so it will be available to Mozilla builds. What won't be available is
the activex plugin.
Status: NEW → ASSIGNED
Flags: blocking1.4?
The patch would have to set --enable-activex-scripting in the configure script,
but also supply patches to lock down the default settings for the functionality
so it did nothing at all. That means activex.js & nsAxSecurityPolicy.js as well
as any internal settings need to assume the strictest values by default.
I think the plugin should still be built if the switch is on, but the package
lists can be changed so it is not actually packaged and shipped.
Reporter | ||
Comment 4•22 years ago
|
||
>IDispatch will be built into xpconnect so it will be available to Mozilla builds.
Yes and no. To clarify, IDispatch code will be there, but there will be no way
to instantiate an ActiveX/COM component via JS code since default security
policty will block any such attempt.
And I agree the patch for activex.js & nsAxSecurityPolicy.js should go here.
As far as how ActiveX plugin is not shiped, either not built or not packaged, I
don't have a preference/opinoin
Assignee | ||
Comment 5•22 years ago
|
||
This patch turns activex-scripting support (IDispatch) on for win32 by default.
It builds but does not export the activex plugin files to dist/.
Assignee | ||
Comment 6•22 years ago
|
||
This patch implements the workaround proposed in bug 127982 so that I can build
with the MS Platform SDK installed.
Reporter | ||
Comment 7•22 years ago
|
||
Comment on attachment 121764 [details] [diff] [review]
IsEqualGUID v1.0
r=dbradley
Thanks for fixing this. There's a couple of instances in the ActiveX code as
well http://lxr.mozilla.org/seamonkey/search?string=InlineIsEqualGUID
Attachment #121764 -
Flags: review+
We still need to ensure activex.js and nsAxSecurityPolicy.js are exported from
plugin/ even if npmozax.dll and nsIMozAxPlugin.xpt are not.
Reporter | ||
Comment 9•22 years ago
|
||
activex.js probably doesn't matter, since if it's not there, the result is a
blank whitelist.
Reporter | ||
Comment 10•22 years ago
|
||
Actually I wonder if it would be better if we could do without
nsAxSecurityPolicy.js as well. If it's not there, we just default to hosting
nothing right?
Comment 11•22 years ago
|
||
I will have to check the code. Certain places hardcode values for hosting flags
in the absence of a service or pref to tell them otherwise. I am sure the
default hosting flags are not absolute and I'll have to check places where the
security service is called to make sure it doesn't fall over with errors when it
is absent.
Comment 12•22 years ago
|
||
We seem to be safe concerning the absence of nsAxSecurityPolicy.js, the
nsDispatchSupport object calls it to return the hosting flags but returns 0 if
there is no service.
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/nsDispatchSupport.cpp#323
The plugin should also be safe since it defaults to 0 when there is no pref but
it needs some work. It asks for the hosting flags directly from the prefs which
is wrong and allows the value to be overridden. Since it grabs the
nsDispatchSupport object nearby it should be calling that for the values. I'll
work a patch to fix this behaviour. I don't think it needs to care about
monitoring the prefs at all in COM connect mode.
http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/plugin/PrefObserver.cpp#59
Reporter | ||
Comment 13•22 years ago
|
||
Since this deals with not shipping DLL, this bug wouldn't be dependant on fixing
issues in the DLL, right? We should be able to go forward with this.
Comment 14•22 years ago
|
||
Yes, the issues in the plugin can be fixed in bug 203704. It looks like the
dispatch support service is happy to return 0 in the hosting flags if the
nsAxSecurityPolicy is not present.
Assignee | ||
Comment 15•22 years ago
|
||
Fixes mingw builds.
Attachment #121763 -
Attachment is obsolete: true
Reporter | ||
Comment 16•22 years ago
|
||
So are we good to go with this, reviews?
Comment 17•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
Does there need to be an entry for MOZ_USE_ACTIVEX_PLUGIN in autoconf.mk.in?
Otherwise r=adamlock
Attachment #121938 -
Flags: review+
Reporter | ||
Comment 18•22 years ago
|
||
Seeking 1.4b blocking status.
We want to get this into beta so that we have some time running with it before
1.4 final.
Reporter | ||
Comment 19•22 years ago
|
||
Seeking 1.4b blocking status.
We want to get this into beta so that we have some time running with it before
1.4 final.
(collision happened and the blocking status got reverted when I went back, sorry
for the extra comment)
Flags: blocking1.4b?
Reporter | ||
Comment 20•22 years ago
|
||
Comment on attachment 121764 [details] [diff] [review]
IsEqualGUID v1.0
Seeking sr/a for small mingw patch so we don't have problems when IDispatch is
built by default
Attachment #121764 -
Flags: superreview?(alecf)
Attachment #121764 -
Flags: approval1.4b?
Reporter | ||
Comment 21•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
Seeking sr to build IDispatch code as part of the default build.
Attachment #121938 -
Flags: superreview?(alecf)
Comment 22•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
sr=alecf but seawood needs to be the reviewer for build-level stuff like
configure.in
Attachment #121938 -
Flags: superreview?(alecf)
Attachment #121938 -
Flags: superreview+
Attachment #121938 -
Flags: review?(seawood)
Attachment #121938 -
Flags: review+
Comment 23•22 years ago
|
||
Comment on attachment 121764 [details] [diff] [review]
IsEqualGUID v1.0
sr=alecf
Attachment #121764 -
Flags: superreview?(alecf) → superreview+
Reporter | ||
Comment 24•22 years ago
|
||
He created the patch, so wasn't sure who would need to review.
Reporter | ||
Comment 25•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
bryner, would you be able to review this. We need to get this reviewed ASAP so
we can get it checked in for 1.4b
Attachment #121938 -
Flags: review?(seawood) → review?(bryner)
Comment 26•22 years ago
|
||
Comment on attachment 121764 [details] [diff] [review]
IsEqualGUID v1.0
a=asa (on behalf of drivers) for checkin to 1.4b
Attachment #121764 -
Flags: approval1.4b? → approval1.4b+
Updated•22 years ago
|
Attachment #121938 -
Flags: review?(bryner) → review+
Reporter | ||
Comment 27•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
Seeking approval for this second patch, this actually turns on building
IDispatch support by default.
Attachment #121938 -
Flags: approval1.4b?
Assignee | ||
Comment 28•22 years ago
|
||
Small add-on to v1.1 to remove bogus dependency from $(SHARED_LIBRARY) rule.
Comment 29•22 years ago
|
||
Comment on attachment 121938 [details] [diff] [review]
v1.1
a=sspitzer
Attachment #121938 -
Flags: approval1.4b? → approval1.4b+
Reporter | ||
Comment 30•22 years ago
|
||
I checked in the patch, but now I'm not sure we're quite where we need to be. I
don't think XPC_IDISPATCH_SUPPORT is getting defined in the default build.
Unfortunately I have that defined in my environment so didn't notice when I was
building with the patch and thought things were working ok.
Reporter | ||
Comment 31•22 years ago
|
||
Ok, after cls set me straight on configure.in and configure, this is now checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 33•22 years ago
|
||
Removing blocking 1.4b since this is checked in
Flags: blocking1.4b?
Flags: blocking1.4?
Comment 34•22 years ago
|
||
Comment from the peanut gallery...I currently build with activex disabled. This
patch now causes my build to break. I get an error from configure.in about
enabling active x scripting support when active x support is disabled.
That seems weird. If I disable activex then activex scripting should be disabled
automatically. Do we really need to say if you don't want active X you need to
set two separate build config options? That doesn't seem efficient.
Why can't disable-activex implicitly disable active-x scripting? Otherwise, what
good is disable-activex?
Assignee | ||
Comment 35•22 years ago
|
||
Because apparently, --disable-activex is only supposed to apply to the activex
control and not everything activex related. We're still hashing out the details
outside of bugzilla.
Comment 36•22 years ago
|
||
This is a new configure patch to make the activex behaviour easier to
understand. Default behaviour is
--enable-activex-control
--enable-activex-scripting
--disable-activex-plugin
The plugin will not be built at all by default, but may be enabled with
--enable-activex-plugin. The --disable-activex-control switch replaces
--disable-activex which was non-obvious with the plugin and scripting
potentially being built as well.
The configure script attempts to check for odd combinations, such as asking for
activex on a unsupported platform.
The plugin's Makefile.in must remove the ifdef MOZ_USE_ACTIVEX_PLUGIN but that
isn't included in the patch.
Comment 37•22 years ago
|
||
Ignore the non ActiveX related changes in that that configure.in file, something
screwed up in my CVS, I'll try and submit a clean patch
Comment 39•22 years ago
|
||
Minor tweak after testing on Linux, to move a Windows specific test before the
plugin dependency on scripting test to ensure a more meaningful error.
Patch also removes the ifdef MOZ_USE_ACTIVEX_PLUGIN in the plugin which should
not be needed any more with this patch.
Attachment #122255 -
Attachment is obsolete: true
Comment 40•22 years ago
|
||
Comment on attachment 122302 [details] [diff] [review]
Configure patch
Chris, can you review this patch to improve the build flags please? Thanks
Attachment #122302 -
Flags: review?(seawood)
Assignee | ||
Comment 41•22 years ago
|
||
Comment on attachment 122302 [details] [diff] [review]
Configure patch
Major confusion here. First, get rid of the MOZ_NO_feature variables.
MOZ_feature is preferred. Second, why is MOZ_NO_ACTIVEX_SUPPORT still around
if the option which controlled it was replaced by one that used
MOZ_NO_ACTIVEX_CONTROL_SUPPORT? I don't see the need for that whole series of
checks just to set MOZ_NO_ACTIVEX_SUPPORT since it's only used in one other
place. In embedding/browser/Makefile.in, it should check to see if any of the
activex variables are set, not just MOZ_NO_ACTIVEX_SUPPORT.
In embedding/browser/activex/src/Makefile.in, you're using the plugin support
variable to check whether to build the control.
Attachment #122302 -
Flags: review?(seawood) → review-
Reporter | ||
Comment 42•22 years ago
|
||
Do we want to reopen this or should we move further patches to the bryner's bug
204292?
You need to log in
before you can comment on or make changes to this bug.
Description
•