264090 - Set property stopped working in scriptable API in PR1

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Vlad Alexander]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10

The following code does not pass values to the plug-in in Firefox PR1:

onclick="document.getElementById('editor1').Value = 'hello world';"

This works okay in Firefox 0.9 and Mozilla 1.8 Alpha 4.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.




For testing, you can use our plug-in. You can download the install program from
this link and it comes with an uninstall:
http://xstandard.com/misc/beta/x-pro.exe

Along with the plug-in, the following test-page will be installed:
c:\xstandard\firefox.htm

To this page, add a button or a link with the following onclick event:
onclick="document.getElementById('editor1').Value = 'hello world';"

Comment 1

[Johnny Stenback (:jst)]

Nice catch! Patch coming up.

Comment 2

[Johnny Stenback (:jst)]

Attachment: Use JS_HasUCProperty() in stead of JS_LookupUCProperty().

This fixes the problem.

Seems like JS_Lookup[UC](Property|Element) is broken. In this case, we resolve
the property "Value", end up finding it on the scriptable object, which defines
the property, setting its value to undefined (JSVAL_VOID), then in the embed
object's setproperty hook we check if the property exists, but we get back
undefined, which means it doesn't exist. If I change the newresolve code in
question to defeine the property to a value other than undefined (e.g.
JSVAL_TRUE), JS_LookupUCProperty() seems to give the expeted result.

Brendan, thoughts? The JS engine code that doesn't seem to do what's desired
here is at http://lxr.mozilla.org/aviarybranch/source/js/src/jsapi.c#2384

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 161936 [details] [diff] [review]
Use JS_HasUCProperty() in stead of JS_LookupUCProperty().

Do you want the branch callback change?

JS_HasUCProperty is the right thing to use here.  See the comment:

	/* XXX bad API: no way to tell "not defined" from "void value" */

a few lines up from the lxr line-link you gave.  API compatibility means
JS_Lookup* can't change, hence the recent JS_Has* API.	Use it in good health. 
I'll lxr for any lingering, incorrect JS_Lookup* uses.

/be

Comment 4

[Johnny Stenback (:jst)]

Fixed on the branches. Not a problem on the trunk.