Closed
Bug 48974
Opened 24 years ago
Closed 24 years ago
Need mechanism for multi-file JS components
Categories
(Core :: XPConnect, defect, P3)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
People
(Reporter: aaronlev, Assigned: rginda)
References
Details
Attachments
(10 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details |
We need the ability to include, or import javascript from inside another
javascript file.
To what context are you referring? In the JS shell (and xpcshell/lcshell
variants), there is a ``load'' command. Are you loading a script into content
with <script src=>? A JS component?
The JS component case is a little trickier, because:
- you have to detect file changes across all included files when you're
figuring out when to re-register (maybe not a problem, long term)
- you have to specify the name of the included file somehow, which gets a
little tricky. If you use an URL (or relative-URL) syntax, you have to have
necko stuff available when you're executing the component script, such as at
registration time. If you use a relative path name, you're constraining
yourself to local files (perhaps not a big deal now; webapp stuff might make it
annoying later), and you have all sorts of XP-file-path issues to face.
I'm not sure what I think about this. Doing ``import'' for JS components and
hoping that relative file paths work -- or waiting for rayw's search-path work
to land? -- is a little tempting, but I fear making a hash of things.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Can't use import statement in Javascript → Need mechanism for multi-file JS components
Yeah, that one covers the in-chrome case pretty thoroughly, but I'm not sure the
issues are the same for JS components, so I'm re-titling this one and keeping it
open.
Assignee | ||
Comment 4•24 years ago
|
||
I'm starting to need this in a bad way for chatzilla. How about a component
with an interface something like...
interface nsIScriptLoader {
bool loadURL (in wstring url);
bool loadFile (in nsIFile file);
};
Comment 5•24 years ago
|
||
Why a new interface (service too, I guess)? If you can use nsILocalFile,
nsIDirectoryService, and eval(), go for it.
/be
Comment 6•24 years ago
|
||
If it doesn't work with relative paths (url or local file) then it is broken.
The last thing we want is people being forced to use absolute paths to load
their library code.
I think I'd prefer a component-context API function, if only so that it could
eventually co-operate with the chrome script cache (or one of its own) to avoid
compiling multiple copies of the same script (IO.js or string-bundle.js, for
example).
Reporter | ||
Comment 8•24 years ago
|
||
The question was put: what context am I using the scripts in?
When designing a large app that will utilize many javascript function
libraries, it is better to list those dependencies within the source javascript
file itself, then in each XUL file. I am not familar with the load() function.
Comment 9•24 years ago
|
||
Aaron, load() works in xpcshell.
load('myLib.js');
xpcshell is part of debug builds.
For now, having a simple load() top level method that automatically deals w/
file url, chrome, local and full path issues would be great.
But if no one has time, then i guess we can do what Brendan suggests.
Perhaps as a top level method w/ a simple API that will handle these details of
the various flavors of local file paths.
So then we can just do a:
include('myLib.js');
This method can also return if any dupe files are passed to it . . .
It probably wouldn't take very long to do.
pete
Assignee | ||
Comment 10•24 years ago
|
||
Assignee | ||
Comment 11•24 years ago
|
||
Assignee | ||
Comment 12•24 years ago
|
||
Here's two whacks at a solution...
iotest.js doesn't associate a filename with the new script, so error messages
would be pretty hard to decipher. Also, you'd have to cut n' paste this into
the first js file that needed to include another js file.
xpc.diff adds wacky dependancies to xpconnect, check out the new #includes in
xpccomponents.cpp (nsIResChannel.h is bogus, I forgot to remove it from the
diff), but you get better error reports. Also, we should probably set some
principals on the new script, and throw compile time errors back to the caller,
instead of returning NS_ERROR_FAILURE.
Comment 13•24 years ago
|
||
Rob, i couldn't get your first example to work.
I added an include function to io.js w/ error checking, etc . . .
However, eval is not working. Anyone know what i am doing wrong here?
Attaching now . . .
pete
Comment 14•24 years ago
|
||
Comment 15•24 years ago
|
||
Here is my test:
js>
load('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/io.js');
js> var file = new File();
js>
file.include('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js');
function pete() { dump("\n\n\n\n********** TEST ***************\n\n\n\n"); }
js> pete();
typein:4: ReferenceError: pete is not defined
Comment 16•24 years ago
|
||
Rob, here is your code error.
I just pasted it in . . .
----------------------------------------------------
js> const IOS_CID = "{9ac9e770-18bc-11d3-9337-00104ba0fd40}";
js> const nsIIOService = Components.interfaces.nsIIOService;
js> const SCRIPTABLEIS_PROGID = "component://netscape/scriptableinputstream";
js> const nsIScriptableInputStream =
Components.interfaces.nsIScriptableInputStream;
js>
js> function loadURL (url)
{
var ios = Components.classesByID[IOS_CID].createInstance(nsIIOService);
var chan = ios.newChannel (url, null);
var is = chan.openInputStream();
var sis =
Components.classes[SCRIPTABLEIS_PROGID].createInstance(nsIScriptableInputStream);
sis.init (is);
var script = sis.read (chan.contentLength);
return eval(script);
}
js>
loadURL('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js');
uncaught exception: [Exception... "Component returned failure code: 0x804b000a
[nsIIOService.newChannel]" nsresult: "0x804b000a (<unknown>)" location: "JS
frame :: typein :: loadURL :: line 9" data: no]
js>
Comment 17•24 years ago
|
||
the onbly way i can get this to work is by doing this:
js> load('/usr/src/M18/mozilla/xpcom/tests/utils/io.js');
js> var file = new File();
js>
eval(file.include('/usr/src/THEME_BUILDER/Theme_Builder/theme_builder/theme_builder/content/js/test.js'));
js> pete();
********** TEST ***************js>
js>
Anyone know why returning an eval doesn't behave as would be expected?
Assignee | ||
Comment 18•24 years ago
|
||
Sorry, I guess I should have specified that these routines load file:,
resource:, and chrome: urls ONLY. If you prefix your path with "file://" it
should work as expected. Though, I don't think you'll be happy about it. Any
errors generated by the included script will be quite hard to track down.
On another note, I spoke with jband about xpc.diff, and since he's too shy to
make his comments here, I'll try to paraphrase...
1) The new dependencies are bad, he'd prefer that we only create the load()
method if the required components exist.
2) jband is morally opposed to checking in anything that can't do relative
paths. He doesn't consider resource: and chrome: as meeting this criteria.
RE point 1, it sounds like we'll end up creating a script-loader component
anyway, so why not just component-ize this code, and leave it to the script
writer to instantiate the component.
RE point 2, IMO, this is a case for Worse is Better, but I'm biased. I'd love
to have this for chatzilla. Relative paths could be added to the component at a
later time w/o affecting current users, who would be using fully qualified
urls. Also, in the context of mozilla, chrome: and resource: urls are relative
enough for most purposes (like, mine for example.) Relative http: paths are
moot, as this routine doesn't load from http:.
If we're willing to apply Allman's law one more time, I think we can avoid the
dependencies without requiring (smaller, granted) boilerplate JS
copied-and-pasted everywhere.
How about the Load::call method tries to load a subscript-loader component by
progID the first time it's called, and then calls through to it if it's able.
If it's not able to create the subscript-loader, it just bails with
NS_ERROR_UNAVAILABLE or whatever.
Our subscript-loader component would be part of the component loader, but only
built/installed ifndef XPCONNECT_STANDALONE, which means that it could also do
the privilege management. Whaddya think?
Hrm. The privilege management thing is actually a little scary in that model.
Maybe the loader should just, ifndef XPCONNECT_STANDALONE, annotate the
Components object with the load() thingy. The loader is already lightly sprayed
with XPCONNECT_STANDALONE conditionals, so what's one more?
Whaddya think?
Comment 20•24 years ago
|
||
For the here and now . .
this is what i have for io.js. It will handle local file paths.
It registers included files so you don't use them again etc. . .
However like i said, i can't get eval to work properly. So i am currently just
retuning the read as a string and using eval from the caller . . .
/********************* INCLUDE **************************/
include : function (path) {
if(!path) {
dump("missing argument\n\n");
return;
}
if(this.leaf(path)=="io.js"){
dump("Sorry, can't include io.js because you are already using it!\n\n");
return;
}
if(!this.exists(path)) {
dump("invalid path\n\n");
return
}
if(this.includeFiles) {
for(var i=0;i<this.includeFiles.length;i++){
if(path==this.includeFiles[i]){
dump("Already using this include file, ignoring . . .\n\n");
return;
}
}
}
this.includeFiles.push(path);
var retval;
this.open(path)
retval = this.read(path);
return retval;
},
/********************* INCLUDE **************************/
Comment 21•24 years ago
|
||
Wouldn't it be better to just use the DOM to do a HTTP GET, and then use an eval
() nested in a try{}?
Comment 22•24 years ago
|
||
Hmm...i misuderstood, you want to load resource: and chrome: .Ignore previous
comment.
Assignee | ||
Comment 23•24 years ago
|
||
So, if we're going to #ifdef XPCONNECT_STANDALONE, why make a new component when
we can just #ifdef out the body of the function (and the includes) and replace
it with a NS_ERROR_UNAVAILABLE. Jband seems to think this would be ok, although
he still thinks including necho stuff in xpconnect is unpure (He half-jokingly
sugested it be a component living in the js-component-loader .so)
Anyway, I'll rework the patch to include the #ifdefs, walk the scope chain
instead of using JS_GetGlobalObject(), and report failures as js exceptions
instead of XPCOM failures, but I'll need help with the principals stuff
(shaver?)
Why was jband half-joking? Let's make the JS component loader put its Load
object in place on the Components object when it bootstraps XPConnect. Localize
the dependencies, etc.
The principal stuff is the other reason; what principal would you give something
that gets Components.load()ed? The principal from the calling script? Let's
make the component loader put that policy in place for its scripts, and maybe
save ourselves a firedrill later on.
Comment 25•24 years ago
|
||
I agree with jband (not joking) and shaver (i think, hope I'm following this all
accurately): we should make a separate component and not wire up necko into a
ball of string with XPConnect.
/be
Assignee | ||
Comment 26•24 years ago
|
||
Making the js component loader strap the load() function to the Components
object (e.g. with JS_DefineFunction) wouldn't expose the load() function to
chrome. This may or may not be a good thing. I'd probably argue the latter.
Turning the code into an nsIXPCScriptable component living the the js component
loader module WFM. XPConnect can attempt to instantiate the component (by
progID, as shaver said) in nsXPCComponents::GetLoad(), and throw an error if
that instantiation fails.
comments?
Assignee | ||
Comment 27•24 years ago
|
||
Assignee | ||
Comment 28•24 years ago
|
||
Assignee | ||
Comment 29•24 years ago
|
||
Assignee | ||
Comment 30•24 years ago
|
||
I used the CLSID instead of the progID, but the rest is my interpretation of
what we've discussed.
Status: NEW → ASSIGNED
I don't care about solving the exposure-to-chrome issue right now.
I chose ``progID'' on purpose, because it would allow XPCONNECT_STANDALONE users
to provide their own load() implementation, which seems like a nice thing.
I'll review your patch now...
Assignee | ||
Comment 32•24 years ago
|
||
ah. Good point. I've changed it to a progID here.
What else? (the new script still has no principals.)
Comment 33•24 years ago
|
||
A few things...
- The reason I was on;ly half serious about putting this code in the jsloader
was because it didn't seem that much better to saddle the loader with this necko
dependency than to saddle xpconnect. But, everyone seems to be fine with this.
whatever.
- Like rginda mentioned, he still needs some principals to compile his script.
This reminds me of somethng that bothers me a lot about this. He supports
passing in a scope object for the compilation. But, we've given JS loader
objects system principals. This means (I think) that if one has the specific
transient privileges to both get access to some JS loader loaded object (e.g.
window.sidebar that *everyone* can get) and to call Components.load, then one
can load code that will end up with system principals and which will never be
checked for privileges at all. This seems broken to me.
- I'm not real clear on why this is Components.anything - i.e. what does a JS
file loader have to do with Components? Why not require JS code to
getService for the loader service and then call 'load'. This allows you to get
rid of the nsIXPCScriptable junk (for which you've added no security check BTW).
- again, I think it sucks that we might leave this as a way to load fully
qualified urls without any mechanism to do relative paths (which take into
account the page (and/or! location of the .js file) from which they are called.
This is in large part a mechanism to allow library code to load other library
code. Needing to know absolute urls (even if they are remapped chrome: urls)
sucks. But, I bend to worse is better arguments as necessary.
Anyway, what is so bad about having people write:
function load(url, obj) {
// XXX get privs here if required
var iface = Components.interfaces.nsIJSSubScriptLoader;
var clazz = Components.classes['whatever'];
var loader = clazz.getService(iface);
return loader.load(url,obj);
}
The service implementation can use nsIXPCCallContext to deal with optional
JSObject param and to get at the JSContext. The security check on whether or not
hte user is allowed to call this particular method of this interface is done
automtically.
Yes, this is more JS code that users would have to write. Is that so bad?
And then... Why not have window.load ? Why should xpconnect privileges come into
play? Shouldn't this really be about a codebase check; i.e. this file can load
that file or not.
I realize that Rob wants to get something working so that he can get on with his
work. But I really don't want to see us institute something that we'll regret.
Group: netscapeconfidential?
Comment 34•24 years ago
|
||
Make that 'window.loader' and it can implement 'load' and whatever else we're
likely to think of in the future. The JS Component loader can have its own
'loader' object hanging off its global. We can add security checks (somehow?) on
the parent object that gets passed in and decide whether or not we want to allow
it to be used as the parent for the code to be compiled. We could even preclude
passing an explicit parent object when called from content JS.
Assignee | ||
Comment 35•24 years ago
|
||
I've reworked this to be a standalone component, included in the jsloader
binary, and using nsIXPCCallContext instead of the scriptable stuff. I'm going
to use it as part of a project I'm playing with and see how it all "feels", I'll
post new patches when it's a little more shaken out.
Depends on: 50447
rginda's gonna do this, it seems. I'm just going to watch. =)
Assignee: shaver → rginda
Status: ASSIGNED → NEW
Assignee | ||
Comment 37•24 years ago
|
||
Accepted bug. Why was this set to "nscp confidential?"... unsetting.
Group: netscapeconfidential?
Status: NEW → ASSIGNED
Comment 38•24 years ago
|
||
Rob, It looks like I toggled that without noticing. I've inadvertanly tabbed to
that box before without catching it right away. I guess I did this here without
noticing at all.
Reporter | ||
Comment 39•24 years ago
|
||
Quick crude way that works
--------------------------
=> Can it be simplified further?
function jsInclude(path)
{
try {
var fileInst = new FilePath(path);
var fileChannel = new FileChannel();
var inputStream = new InputStream();
fileChannel.init(fileInst, 0x01, fileInst.permissions); /* Open to read */
var inStream = fileChannel.openInputStream();
inputStream.init(inStream);
eval(inputStream.read(fileInst.fileSize));
inStream.close();
}
catch (e) {
dump("jsInclude ERROR: "+e+"\n\n");
}
}
jsInclude("includethis1.js");
Reporter | ||
Comment 40•24 years ago
|
||
Oops - forgot to set these consts at the top.
Here's the whole simple solution again.
/////////////////////////////////////////////////////////////////////////
const FilePath = new Components.Constructor( "@mozilla.org/file/local;1",
"nsILocalFile", "initWithPath");
const FileChannel = new Components.Constructor(
"@mozilla.org/network/local-file-channel;1", "nsIFileChannel" );
const InputStream = new Components.Constructor(
"@mozilla.org/scriptableinputstream;1", "nsIScriptableInputStream" );
function jsInclude(path)
{
try {
var fileInst = new FilePath(path);
var fileChannel = new FileChannel();
var inputStream = new InputStream();
fileChannel.init(fileInst, 0x01, fileInst.permissions); /* Open for reading */
var inStream = fileChannel.openInputStream();
inputStream.init(inStream);
eval(inputStream.read(fileInst.fileSize));
inStream.close();
}
catch (e) {
dump("jsInclude ERROR: "+e+"\n\n");
}
}
jsInclude("/home/aaron/fred.js");
Reporter | ||
Comment 41•24 years ago
|
||
Sorry for the bugspam, one more correction:
Make jsInclude return a string of what you want to eval outside of jsInclude.
So you'll end up doing:
eval(jsInclude("include-me.js"));
Otherwise all of your functions and consts you define will be destroyed after
jsInclude returns, because of scope.
Reporter | ||
Comment 42•24 years ago
|
||
I'm having 2 problems with the read-file/eval it method just derscribed:
1. Javascript warnings and error messages give the wrong file and line numbers,
so problems are much harder to track down.
2. I have the main component in dist/bin/components, but where should I store
the include files? I don't want them in the same directory, otherwise they get
initialized on startup.
Reporter | ||
Comment 43•24 years ago
|
||
Solution to file location.
As suggested by John Bandauer, I named the include files with a .jsl extension
(javascript library file) and stuck them in the components directory along with
the .js component that uses them.
I also had to change the js code that includes them to now find the proper
directory that they're in.
We still have the problem with the line numbers being incorrect in error/warning
messages.
Here's the complete code, that includes files from the components directory:
////////////////////////////////////////////////////////////////////////////////////////////
// Include javascript files - this works until a better multi-file js mechanism
is achieved
/////////////////////////////////////////////////////////////////////////////////////////////
// First get include directory name
var dirServiceProvider =
Components.classes["@mozilla.org/file/directory_service;1"].
getService().QueryInterface(Components.interfaces.nsIDirectoryServiceProvider);
var persistent = new Object();
var includeDir = dirServiceProvider.getFile("ComsD", persistent).path;
// This returns the include file from the include directory as a string
function jsInclude(includeFileName)
{
var path=includeDir+"/"+includeFileName;
const FilePath = new Components.Constructor( "@mozilla.org/file/local;1",
"nsILocalFile", "initWithPath");
const FileChannel = new Components.Constructor(
"@mozilla.org/network/local-file-channel;1", "nsIFileChannel" );
const InputStream = new Components.Constructor(
"@mozilla.org/scriptableinputstream;1", "nsIScriptableInputStream" );
try {
var fileInst = new FilePath(path);
var fileChannel = new FileChannel();
var inputStream = new InputStream();
fileInst.permissions=0x0755;
const READ_MODE=0x01;
fileChannel.init(fileInst, READ_MODE, fileInst.permissions); /* Open for
reading */
var inStream = fileChannel.openInputStream();
inputStream.init(inStream);
var ret=inputStream.read(fileInst.fileSize);
inStream.close();
}
catch (e) {
dump("jsInclude ERROR: "+e+"\n\n");
return "";
}
return ret;
}
// Now go get the files and eval them
eval(jsInclude("nsAccessibleState.jsl"));
eval(jsInclude("nsAccessibleCommands.jsl"));
//////////////////////////////////////////////////////////////////////////
Assignee | ||
Comment 44•24 years ago
|
||
It's been a while, but better late than never, I guess.
Attached after this comment is a patch that adds a new component,
"@mozilla.org/moz/jssubscript-loader;1". The component lives in the
js-component-loader library. It has a single method, loadSubScript(), that
takes as parameters a URL to load, and optionally (through the magic of
nsIXPCNativeCallContext) an object to use as a global during the eval. The URL
*must* refer to a local file, and use a file:, resource:, or chrome: scheme.
This component is only intended to be called from JavaScript code.
Loaded scripts are given the system principals, and no caching of compiled
scripts is done.
Because the loader adds dependencies on nsIChannel and nsIIOService, it can be
removed from the build by #define-ing NO_SUBSCRIPT_LOADER or XPCONNECT_STANDALONE.
Attached is the main file, mozJSSubScriptLoader.cpp for online review, and a
.zip of the entire mozilla/js/src/xpconnect/loader subdirectory for more
complete review and testing.
Mac build goop still needs to be added, I'll need some help there.
looking for some hot s?r= action.
Assignee | ||
Comment 45•24 years ago
|
||
Assignee | ||
Comment 46•24 years ago
|
||
Assignee | ||
Comment 47•24 years ago
|
||
um, oops. I attached the wrong .cpp file, here comes the right one.
Assignee | ||
Comment 48•24 years ago
|
||
I dig it with a vengeance, and want it for 0.8.1. sr=shaver, let's get some r=
and slip this baby in.
Assignee | ||
Comment 50•24 years ago
|
||
attaching updated mozJSSubScriptLoader.cpp, fixes include:
NS_WITH_SERVICE->do_GetService changes
text for null |buf|
In addition, I'll be chkecing the idl file into xpconnect/idl, instead of
xpconnect/loader/idl.
The 0.8.1 deadline is today! Still looking for a r=, jband, brendan, bueller?
adding peterv to cc list for mac build mods.
Assignee | ||
Comment 51•24 years ago
|
||
Assignee | ||
Comment 52•24 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•