424019 - NPN_RemoveProperty returns incorrect result when applied to DOM elements

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Kenneth Russell]

When NPN_RemoveProperty is applied to a property of a DOM element (such as the "value" property of an HTML form input field), the return result is "true", indicating that the removal occurred successfully, but in fact the operation does not succeed.

This is related to bug 411040, though it appears that the fix for that bug did not cover this case.

Please see the attached test case and try it with the new Java Plug-In from http://jdk6.dev.java.net/6uNea.html . On IE, the JSObject removeMember operation fails, and the code detects and handles this. On Firefox, the JSObject removeMember operation claims that it doesn't fail, but the "value" property of the HTML input field is unmodified.

We aren't requesting that the removeMember operation actually mutate the DOM, only that NPN_RemoveProperty return false in this case.

Comment 1

[Kenneth Russell]

Attachment: Test case

Test case

Comment 2

[Kenneth Russell]

Note that the Java Console should be opened via the Advanced tab in the Java Control Panel to see the printed output from the above test case.

Comment 3

[Johnny Stenback (:jst)]

We should fix this for 1.9. What's happening here is that the propery is properly being deleted, and we claim it was deleted, but the property in question is a DOM property exposed through XPConnect, so after it's been deleted, it simply gets added back the next time someone looks for it. This is trivial to fix by simply looking up the property after it was deleted and using that to determine if the property really got deleted.

Comment 4

[Kenneth Russell]

Out of curiosity, why does the value remain unchanged after the deletion and re-addition of the property? Is the value cached in the XPConnect layer?

Comment 5

[Johnny Stenback (:jst)]

The property you're deleting is merely the JS getter/setter for the value, the actual value lives in the C++ implementation behind the JS object, and thus, the value is unaffected by deleting the JS getter and setter for the property in question.

Comment 6

[Johnny Stenback (:jst)]

Attachment: Updated testcase.

Kenneth, the testcase doesn't appear to work, but changing JSObjectRemoveMember to JSObjectDOMRemoveMember in the "code" applet attribute in the HTML file appears to fix it. Here's an updated testcase.

Comment 7

[Johnny Stenback (:jst)]

Attachment: Fix.

This fixes this problem by re-resolving the property after it's been deleted to check if it's a property that gets re-created when resolving it again.

Comment 8

[Kenneth Russell]

(In reply to comment #6)
> 
> Kenneth, the testcase doesn't appear to work, but changing JSObjectRemoveMember
> to JSObjectDOMRemoveMember in the "code" applet attribute in the HTML file
> appears to fix it. Here's an updated testcase.

I'm really sorry about that. I must have made a mistake copying it between machines.

Comment 9

[Johnny Stenback (:jst)]

No problem, it was an easy fix :)

Comment 10

[Brendan Eich [:brendan]]

Wouldn't it be better to make such properties JSPROP_PERMANENT?

/be

Comment 11

[Kenneth Russell]

Note that it's no problem from our side if the properties can't be removed -- this would be more consistent with how IE behaves, anyway.

Comment 12

[Johnny Stenback (:jst)]

(In reply to comment #10)
> Wouldn't it be better to make such properties JSPROP_PERMANENT?

Yeah, it would, but I don't want to make such a change now, at least not w/o a beta. I'd rather do this for now and file a followup bug on this for later.

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 311833 [details] [diff] [review]
Fix.

Please file a followup bug and reference it in a FIXME: nnnnnn comment here. Thanks,

/be

Comment 14

[Johnny Stenback (:jst)]

Filed bug 425823 and checked this in with comments added referencing that bug.