objects created with GeckoActiveXObject are broken

RESOLVED FIXED

Status

()

Core
XPConnect
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: David Bradley, Assigned: David Bradley)

Tracking

({fixed1.8, regression})

Trunk
x86
Windows XP
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8rc1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
The patch in bug 273953 apparently broken the IDispatch support. It's testing
for a JSObject on the tearoff that is found and bailing if one doesn't exist. I
don't believe this test is necessary. I suspect the original code was calling
FindTearOff with a value of true for needsJSobject and everything was ok. I
didn't provide that for LocateTearOff because I didn't think that was necessary.
Unfortunately I forgot the remove the subsequent test in the caller.

Many thanks to Garret Davis for tracking this down.
(Assignee)

Comment 1

12 years ago
Created attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

Looking for reviews
Attachment #198991 - Flags: superreview?(brendan)
Attachment #198991 - Flags: review?(jst)
(Assignee)

Updated

12 years ago
Blocks: 310097

Comment 2

12 years ago
(In reply to comment #1)
> Created an attachment (id=198991) [edit]
> Removes the unecessary JSObject test
> 
> Looking for reviews

I have built and tested this patch in the latest trunk and have verified that
objects created with GeckoActiveXObject are no longer broken.
Need to fix this for 1.8 / Firefox 1.5, for embedding opportunities/partners who
need to whitelist ActiveX plugins and ship the glue.

/be
Flags: blocking1.8rc1+
Keywords: regression
Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

sr=me, in advance of jst's, to get this moving toward branch approval.

/be
Attachment #198991 - Flags: superreview?(brendan) → superreview+

Updated

12 years ago
Whiteboard: [needs review jst]

Comment 5

12 years ago
jst, can you review this soon for us? Thanks!
Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

r=jst
Attachment #198991 - Flags: review?(jst) → review+
Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

dbradley, can you land this on the trunk ASAP?	Thanks,

/be
Attachment #198991 - Flags: approval1.8rc1?
(Assignee)

Comment 8

12 years ago
I'll have some time this evening.
Status: NEW → ASSIGNED
(Assignee)

Comment 9

12 years ago
Patch checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Ok, this may not get trunk testing just by being checked in.  Can people who do
test it verify the bug, and we can get the patch plus'ed for 1.8 branch checkin?
 Thanks,

/be

Comment 11

12 years ago
David and Garrett, can you guys verify this? We're coming into the final days of
the release and need to have all changes in ASAP. Thanks.
(Assignee)

Comment 12

12 years ago
My Cygwin install is botched, tried to get it working last night without luck.
I'm going to try again and get a good build this evening. If someone else can
verify this, that would be great, otherwise I'll try again tonight.

Comment 13

12 years ago
(In reply to comment #11)
> David and Garrett, can you guys verify this? We're coming into the final days of
> the release and need to have all changes in ASAP. Thanks.

I have verified this on MOZILLA_1_8_BRANCH. I can now call properties and
methods on GeckoActiveXObject's using firefox.exe on the 1.8 branch.
I'm pretty sure this was *not* checked in on the 1.8 branch...

Comment 15

12 years ago
(In reply to comment #14)
> I'm pretty sure this was *not* checked in on the 1.8 branch...

You're correct, it wasn't checked in.  I verified the patch by applying the
patch to 1.8, building, and testing.  I'm sorry I wasn't specific.
ah, ok ;-)

Updated

12 years ago
Attachment #198991 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 17

12 years ago
I landed this on the 1.8 branch tonight for David.
Keywords: fixed1.8
Whiteboard: [needs review jst]
You need to log in before you can comment on or make changes to this bug.