Overview Description: DOM can't get the attributes of the APPLET in the attached testcase. When I used getElementsByTagName to access the element, JavaScript stopped executing. Steps to Reproduce: - See the attached testcase Actual Results: Find the APPLET : [object HTMLCollection] APPLET height - using document.body.childNodes[2].height: undefined APPLET width - using getElementsByTagName : Expected Results: Find the APPLET : [object HTMLCollection] APPLET height - using document.body.childNodes[2].height: 30 APPLET width - using getElementsByTagName : 300 APPLET align - using getElementsByTagName : Right Build Date & Platform Bug Found: 2000-11-08-01-MN6: Win32 2000-11-08-00-MN6: Mac 2000-11-08-07-MN6: Linux

Didn't mean to say "attached" test case. Use the URL: http://bubblegum/ngdriver/suites/Bugs/applet_bug.html

The patch I just attached makes applet elements usable from JS even if the JS plugin is not installed but I don't happen to have it here now so I couldn't test with the plugin, need to test that before going further with this bug...

Ok, so I tried this with the Java plugin installed and things still look better but something is still wrong. The script object that's returned as the script object for the applet element is a wrapper for the java plugin object and the prototype of the wrapper is set to the normal DOM JS object. Accessing appler.width should work (IMO) since the prototype (the DOM object) has a width property but doing this fails with the message 'Java class parbanner has no public field or method named "width"' so it seems like property access is not delegated to the prototype of the wrapper in this case. Cc:ing Brendan and beard for more input, beard, you wrote the wrapping code, any idea why it doesn't work? Or did I misunderstand something here?

I can reproduce this problem on the Mac as well, with the MRJ plugin. I added these lines to the test case, explicitly walking up the prototype chain, and still see the height property as undefined: var applet = document.body.childNodes[2]; var proto = applet.__proto__; document.write("<BR>APPLET height - using applet.__proto__.height: "); document.write(proto.height); I'll attached the modified test case as a .zip file.

Ooops, that last test was bogus, if I walk the applet's __proto__ field explicitly, I can read the width and align properties, and then it works from the applet itself! Enclosing better test case.

The problem seems to be that JS_LookupProperty doesn't always return the proper value, perhaps it's some kind of caching issue. Changing the call to JS_LookupProperty seems to fix the problem, so that most of the test case works.

Brendan, perhaps we should be calling JS_LookupProperty if lookup_member_by_id is called by JavaObject_lookupProperty, and JS_GetProperty if called by JavaObject_getPropertyById? This is all in jsj_JavaObject.c.

Nit: put named_prop_op before lookup_member_by_id's out params, that's the fur and brendan style (in before inout before out). You don't want NULL to be a valid input for named_prop_op, because the call from JavaObject_setPropertyById needs to check the prototype chain too. That call should pass JS_LookupProperty too. /be

beard came by my other cube and pointed out that JavaObject_setPropertyById passes NULL for vp to lookup_member_by_id, which defeats the prototype-chain lookup-or-get therein. But I countered that the subsequent code at http://lxr.mozilla.org/mozilla/source/js/src/jsj_JavaObject.c#659 handles the !member_descriptor case, which must be the "I found something in a prototype" case. But I didn't study that case enough at a glance to see that it horribly special-cases "__proto__". Better idea: have lookup_member_by_id call JS_GetPropertyAttributes rather than named_prop_op on proto_chain, and return the attrs via a uintN *attrsp param to callers, if the property is found according to the foundp out param that JS_GetPropertyAttributes takes. Then, - in JavaObject_getPropertyById, call JS_GetProperty in the !member_descriptor early return true case; - in JavaObject_setPropertyById, check in the !member_descriptor case whether (attrs & JSPROP_SHARED); if so, call JS_SetProperty on obj and let the shared magic happen; - in JavaObject_lookupProperty, fix the bogus TODO-commented code by calling JS_LookupProperty if !member_descriptor, which will set *objp correctly. Oh, it looks like fur didn't follow the "in before inout before out params" style rule in lookup_member_by_id [no big, I'm just anal] because id, an in param, comes after java_wrapperp, an out param. Oh well, I still think putting named_prop_op earlier in the param list would be better, but if you buy my line above, there is no need for named_prop_op any longer -- it gets replaced by an attrsp out param. /be

QA contact Update

Beard, AFAIK the DOM side of this bug is fixed, wanna take this from me and fix the remaining problems in LiveConnect? Setting target milestone to mozilla0.9.1

moving to TM of 0.9.2 per PDT triage (you can check it into 0.9.1 until Friday, 18/May/01 or into 0.9.2 after the tree opens)

Patrick, all the DOM issues in this bug should be fixed now and all the remaining problems should be liveconnect problems, reassigning to you.

Updating QA contact to Shivakiran Tummala.

--> mozilla1.0*

applets width, height and align cannot be retrieved using getElementsByTagName...i think this is feature is important and should be taken care of ....what do u guys think

Is this fixed yet? These testcase still don't work: http://bubblegum.mcom.com/ngdriver/suites/dom1/hapl001.html http://bubblegum.mcom.com/ngdriver/suites/dom1/hapl002.html http://bubblegum.mcom.com/ngdriver/suites/dom1/hapl003.html http://bubblegum.mcom.com/ngdriver/suites/dom1/hapl004.html

no this one is not fixed yet. dom still cannot read applet attributes.

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)

nominating for nsbeta1.

I've retargeted the milestone and plussed this one per triage with ADT.

var foo = document.createElement('Applet'); foo.name='myapplet'; foo.archive = 'mytest.jar'; foo.code='mytest.class'; foo.mayscript=true; foo.width = "100"; foo.height = "100"; document.body.appendChild(foo); We are using only javascript/dom to create web application, so we are also using this to create an applets and append it to the body. This works for ie but not for mozilla. Please make sure that this is also gonna work. Keep attention that we also use the mayscript attribute. For now (mozilla 0.9.8+) will not find the jar and class file. And has problem with the mayscript attribute (this may be an other story:-)

Olaf, that's bug 33111

With this patch, the test case works, except that there appears to be a bug in the test case itself, the applet element isn't accessible as |document.body.childNodes[2]| but instead as |document.body.childNodes[3]|. May I have some reviews?

By the way, the URLs in comment #22 don't work for me. Have they been moved?

In comment #14, Brendan wrote: >Better idea: have lookup_member_by_id call JS_GetPropertyAttributes rather than >named_prop_op on proto_chain, and return the attrs via a uintN *attrsp param to >callers, if the property is found according to the foundp out param that >JS_GetPropertyAttributes takes. Then, > >- in JavaObject_getPropertyById, call JS_GetProperty in the !member_descriptor >early return true case; >- in JavaObject_setPropertyById, check in the !member_descriptor case whether >(attrs & JSPROP_SHARED); if so, call JS_SetProperty on obj and let the shared >magic happen; >- in JavaObject_lookupProperty, fix the bogus TODO-commented code by calling >JS_LookupProperty if !member_descriptor, which will set *objp correctly. My next patch makes a stab at implementing this. I've removed the named_prop_op parameter, and added an out parameter that returns the prototype of the object, and a struct parameter of type JSPropertyInfo that records the name and JSPROP_* attributes of the property. JavaObject_getPropertyById now checks to see if the property is defined in the prototype chain, and uses JS_GetProperty to read it from there if it is. JavaObject_setPropertyById now also checks to see if the property is defined in the prototype chain, and if it is marked as shared, JS_SetProperty is called to perform the shared magic. Finally, in JavaObject_lookupProperty, if the propery is defined in the prototype chain, *objp is set to the prototype chain, rather than the object itself. You mentioned using JS_LookupProperty, but I don't see how it will really help. Please review my next patch.

Comment on attachment 71518 [details] [diff] [review] Fix out parameter initialization problem. >+struct JSPropertyInfo { >+ const char* name; >+ uintN attributes; >+}; >+typedef struct JSPropertyInfo JSPropertyInfo; Does that want to be JSJPropertyInfo, to signify that it's not a part of the JS engine proper? >@@ -678,4 +707,5 @@ > } > JS_SetPrototype(cx, obj, JSVAL_TO_OBJECT(*vp)); >+ } > jsj_ExitJava(jsj_env); > return JS_TRUE; Is this a -w diff, or do you need to reindent a bit? sr=shaver, please reindent as required and consider the JSPropertyInfo->JSJPropertyInfo renameing.

This is a -w patch, and I have completely reindented the file (removed hard tabs). I'll make the typedef name change.

Comment on attachment 71518 [details] [diff] [review] Fix out parameter initialization problem. r=rogerl, indenting was the only thing I had found, darn it.

Awaiting approval to check in.

Eeek, I missed an else-after-goto: + *proto_chainp = proto_chain; + prop_infop->name = member_name; goto done; + } else { + *proto_chainp = NULL; Lose the else, please!

Final patch.

Comment on attachment 72027 [details] [diff] [review] Removed else, white space diffing a=asa (on behalf of drivers) for checkin to 0.9.9 and the mozilla trunk. also bringing reviews forward.

Fix checked into trunk.

Fix checked into 099 branch.

Reopening bug. all the attributes of the applet are not read (says undefined). Build: 2002-03-07-10-trunk win2k. Changing priority to P2. URL for testcases: mozilla.org/quality/ngdriver/suites/dom-html/hapl0[1-11].html

I tried all of the test cases on this page: http://mozilla.org/quality/ngdriver/suites/dom-html/ Here are the test cases that failed: hapl004.html failed because, IN FACT, the CODE attribute is "parbanner.class" not "NervousText.class" hapl008.html does fail, not sure why. hapl009.html failed. The test case is invalid, because it is reading an attribute called |object| when it should be reading an attribute called | code|. The one distinguishing difference in hapl008.html is that the <APPLET> element has a |name="MarvinRocks" | attribute which makes the applet object visible to the document as: document.MarvinRocks In all of the rest of the test cases, there is no |name| attribute. It appears that if there is a |name| attribute, Java methods are visible, such as: javascript:alert(document.getElementsByTagName('APPLET').item(0).get Class()) This works in hapl008.html, but not in any of the other test cases. Is it supposed to be the case that the only way the Java applet's methods are visible in the DOM object is when the element has a |name| attribute?

Stranger still, in the build I'm testing, Gecko/20020310, the FIRST time I load hapl008.html, the test passes, but subsequent loads all fail. Time to break out a debugger on this.

I created an other test case which may be intresting for you guys. http://technetos.ch:8080/technetos/StyleToApplet.html There is a big diffrence between an none appended applet (to the html body) and an appended one. Just take a look at it, it's maybe helping you.

This fixes part of this problem, according to beard there's still a problem in LiveConnet and he's working on a fix for that...

*** Bug 132179 has been marked as a duplicate of this bug. ***

I thought it'd be clearer if I applied the patch and tweaked the code, then diffed and attached. Here are the changes: - "re-enterd" typo fixed. - Object.prototype used throughout to denote the base prototype object in JS, instead of Object (which is the constructor for base objects, whose .prototype property refers to the base prototype; Object.prototype.constructor == Object). - Preserved the "xpc wrapped native pi (plugin instance)" label -- it confused me that this changed to "pi's JavaScript object (wrapper?)" -- why the wrapper? parenthetical question? Anyway, I was gonna sr= but wanted to pass along these nit-picky changes in case any make sense. /be

The reason for taking out the references to xpc wrappers n' such in those comments was that in some cases those JS objects are in fact not xpc wrappers. In the applet case, for instance, they'll be objects wrapped by LiveConnect, and not by XPConnect. In reality, the first example of what the proto chain would look like after we're done setting up the proto chain all xpc wrapper references in the old code are valid, but in the other sample where the plugin didn't provide a proto (or gave us Object.prototype) the plugin object is not a xpc wrapper.

The previous checkin didn't properly delegate to the prototype in some cases when implementing the JSObjectOps lookupProperty, getProperty, and setProperty. Most significantly, this patch ensures that JavaObject_lookupProperty() only returns a JSProperty* "flag" value when the property exists in the Java object itself.

Comment on attachment 75236 [details] [diff] [review] [checked in on trunk] Haln-n-half of the two above diffs Thanks, that's clearer (the parenthetical asides ending in ? sowed doubt, now the comment describes the cases). sr=brendan@mozilla.org /be

I'll file a separate performance bug on the issue of multiple calls to JS_GetProperty/JS_GetPropertyAttributes. This patch should "do the right thing."

lookup_member_by_id() returns JS_TRUE if the property is found EITHER in the Java object OR in the prototype. Use proto_chain to distinguish that.

Comment on attachment 75323 [details] [diff] [review] REALLY take brendan's comments into account. >@@ -725,11 +725,11 @@ > } > > /* Could be assignment to magic JS __proto__ property rather than a Java field */ > if (!member_descriptor) { > if (proto_chain && (prop_info.attributes & JSPROP_SHARED)) { >- JS_SetProperty(cx, proto_chain, prop_info.name, vp); >+ OBJ_SET_PROPERTY(cx, proto_chain, id, vp); Time to check this for false (failure/error/exception) return value? I don't think it can hurt, because anyone counting on success in spite of a JS_SetProperty failure will get an incoherent combination of true return from this function, and a pending exception stuck in cx. >@@ -786,32 +786,39 @@ > JSErrorReporter old_reporter; > jsval dummy_val; > JSObject *proto_chain; > JSJPropertyInfo prop_info; > JSJavaThreadState *jsj_env; >+ JSBool found; Canonical name is ok, right? If lookup_member_by_id returns false, it reported/threw, I think we noted. > > /* printf("In JavaObject_lookupProperty()\n"); */ > > /* Get the Java per-thread environment pointer for this JSContext */ > jsj_env = jsj_EnterJava(cx, &jEnv); > if (!jEnv) > return JS_FALSE; >- > old_reporter = JS_SetErrorReporter(cx, NULL); >- if (lookup_member_by_id(cx, jEnv, obj, NULL, id, NULL, &dummy_val, >- &proto_chain, &prop_info)) { >- /* signify that the property is in the prototype chain or the object itself. */ >- *objp = (proto_chain ? proto_chain : obj); >+ >+ found = lookup_member_by_id(cx, jEnv, obj, NULL, id, NULL, &dummy_val, >+ &proto_chain, &prop_info); >+ >+ JS_SetErrorReporter(cx, old_reporter); >+ jsj_ExitJava(jsj_env); >+ >+ if (found) { >+ if (proto_chain) { >+ /* Delegate property lookup to object's prototype. */ >+ return OBJ_LOOKUP_PROPERTY(cx, proto_chain, id, objp, propp); >+ } >+ *objp = obj; > *propp = (JSProperty*)1; >+ return JS_TRUE; > } else { else after return, wahhhh. > *objp = NULL; > *propp = NULL; >+ return JS_FALSE; > } Really, how about using ok not found, and doing if (!ok) return false in effect (and not clearing out params before false return). That's SOP in JS-land. Pick these minor issues and sr=brendan@mozilla.org. /be

Comment on attachment 75705 [details] [diff] [review] Picked brendan's nits, minor white space corrections. Move brendan's sr forward.

Doncha want to check for set-property failure here: @@ -725,11 +725,11 @@ } /* Could be assignment to magic JS __proto__ property rather than a Java field */ if (!member_descriptor) { if (proto_chain && (prop_info.attributes & JSPROP_SHARED)) { - JS_SetProperty(cx, proto_chain, prop_info.name, vp); + OBJ_SET_PROPERTY(cx, proto_chain, id, vp); /be

After chatting with jband a little bit, I realize I want to change how lookup_member_by_id works pretty fundamentally. Since LiveConnect now lives in a world where it is in a deep prototype chain, I don't want lookup_member_by_id to signal an error when a member doesn't exist, so I want it to return JS_TRUE in almost all cases, and have callers delegate to a prototype whenever possible. New patch imminent.

Comment on attachment 72027 [details] [diff] [review] Removed else, white space diffing obsoleting old patch that has already landed so it doesn't look like an approved change not yet checked in.

http://technetos.ch:8080/technetos/StyleToApplet.html still does not work as it should with build 2002052306 (rc3) tested on win32. question: how should/does mozilla handle this? Create an applet object by javascript. Befor appending to the body, the applet object seems to be an javascript object, and therefore the style property works and returns a valid javascript style object. After appending the applet object to the body it's a java (applet) object and the javascript object functions/properties are lost (e.g style is not a valid property anymore). This is the same with the name property (which is a javascript object property and not a java applet class property). Please look at the example in the StyleToApplet.html.

Comment on attachment 75236 [details] [diff] [review] [checked in on trunk] Haln-n-half of the two above diffs r=bzbarsky on the classinfo changes

Comment on attachment 75236 [details] [diff] [review] [checked in on trunk] Haln-n-half of the two above diffs This patch was checked in on the trunk.

This will have to get pushed out, while I'm on sabbatical. jband and I have had several hour long conversations about how LiveConnect's JSClass support should probably be rewritten. Because of this, it is difficult to see exactly how to proceed.

*** Bug 154692 has been marked as a duplicate of this bug. ***

transferring topcrash info from bug 154692

renominating, too.

*** Bug 145259 has been marked as a duplicate of this bug. ***

Making this topcrash+ since we have a testcase and a patch ready. Actually, according to comment #63, this fix was checked into the Trunk a while ago. Should this be marked fixed?

->peterl

per triage, based on comment #64 looks like Liveconnect needs to be rewritten and it looks like the crash has been fixed, topcrash-, nsbeta1-, Future, P3

nsbeta1- based on comment #71

Making topcrash- per latest Talkback data. I was about to mark this worksforme, but leaving open based on comment #71.

no longer useful.

I'm pretty sure this still happens, it's just that the page-info code has a workaround to be carefull around applet-attributes.

I'd say any remaining bugs should be filed as follow-ups; this bug has too much information that's no longer directly relevant.

Actaully, reopening till those followups are filed. Note bug 160959 in particular; that may make it difficult to reproduce this one, but backing out the hacks in page info should make you crash just fine.

(In reply to comment #8) > Created an attachment (id=21890) [details] > Better test case which walks correct prototype chain. The crash from this testcase still occurs in Fx mozilla-central latest-trunk nightly in WinXP SP3. !exploitable doesn't seem to offer anything useful. 1:027> !exploitable -v HostMachine\HostUser Executing Processor Architecture is x86 Debuggee is in User Mode Debuggee is a live user mode debugging session on the local machine Event Type: Exception Exception Faulting Address: 0x7c90120e First Chance Exception Type: STATUS_BREAKPOINT (0x80000003) Faulting Instruction:7c90120e int 3 Basic Block: 7c90120e int 3 Exception Hash (Major/Minor): 0x3f6c3744.0x35263758 Stack Trace: ntdll!DbgBreakPoint+0x0 ntdll!LdrpInitializeProcess+0xffa ntdll!_LdrpInitialize+0x183 ntdll!KiUserApcDispatcher+0x7 Instruction Address: 0x7c90120e Description: Breakpoint Short Description: Breakpoint Exploitability Classification: UNKNOWN Recommended Bug Title: Breakpoint starting at ntdll!DbgBreakPoint+0x0 (Hash=0x3f6c3744.0x35263758) While a breakpoint itself is probably not exploitable, it may also be an indication that an attacker is testing a target. In either case breakpoints should not exist in production code.

This is so old and some kind of fix was checked in. The signature we are seeing now is probably a different cause and we should log a new bug. The volume is really low also. If there are new issues, we should file a new bug. Per comment, resolving as works for me.