can we move nsIScriptContext(Owner) from dom to xpconnect

RESOLVED FIXED

Status

()

Core
DOM
--
enhancement
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: timeless, Assigned: markh)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
here's the deal:

jsd shouldn't require dom, it's just a debugger, jsd natively requries js, and jsd_xpc requires xpconnect. requiring dom is annoying (e.g. for hannibal who's trying to play with it).

jsd_xpc uses nsIScriptContext which currently lives in dom.

i'd like to move nsIScriptContext out of dom (xpconnect seems nice to me).

until markh landed nsIScriptContext was dom agnostic, now it actually needs dom. i'd like to revert that change (it's sufficient in my book to change the DidSetDocument function to take an nsISupports instead of nsIDOMDocument).
(Assignee)

Comment 1

12 years ago
I'm not sure xpconnect is the correct place - xpconnect is JS specific (assuming we are talking about the same xpconnect - the one under js/src/xpconnect)

OTOH, you seem to be saying nothing needs to move, but DidSetDocument can use nsISupports.  This option seems easier and totally works for me (I've switched between nsIDOMDocument and nsIDocument a few times due to build issues...)

I'm happy to change DidSetDocument to nsISupports though - please check out the following patch.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

12 years ago
Created attachment 228939 [details] [diff] [review]
DidSetDocument takes an nsISupports
Attachment #228939 - Flags: review?(timeless)
(Reporter)

Comment 3

12 years ago
Comment on attachment 228939 [details] [diff] [review]
DidSetDocument takes an nsISupports

fine by me. personally i wonder why {;} instead of ; and actually implementing it as empty in whichever other non python impls exist. seems like you're cheating a bit.

w/ this change, we can arrange for the interface to move if everyone agrees.
Attachment #228939 - Flags: superreview?(jst)
Attachment #228939 - Flags: review?(timeless)
Attachment #228939 - Flags: review+
Comment on attachment 228939 [details] [diff] [review]
DidSetDocument takes an nsISupports

This patch seems to be missing the same change to nsIScriptContext.h (I'm assuming you just didn't diff that file).

sr=jst with that issue addressed.
Attachment #228939 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 5

12 years ago
thanks jst - nsIScriptContext was indeed just missing from the patch (but I'm glad you pointed it out, otherwise I may have missed checking it in!)

Checking in dom/public/nsIScriptContext.h;
new revision: 3.67; previous revision: 3.66
Checking in dom/src/base/nsJSEnvironment.h;
new revision: 1.79; previous revision: 1.78
Checking in extensions/python/dom/src/nsPyContext.cpp;
new revision: 1.3; previous revision: 1.2
Checking in extensions/python/dom/src/nsPyContext.h;
new revision: 1.3; previous revision: 1.2
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

8 years ago
Assignee: general → mhammond
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.