725448 - Documentation should not suggest using onclick property from content scripts

Status: RESOLVED INVALID

(Add-on SDK Graveyard :: Documentation, defect, P1)

Description

[Wladimir Palant]

I noticed that https://addons.mozilla.org/en-US/developers/docs/sdk/1.4/dev-guide/addon-development/content-scripts/access.html explains how content scripts access DOM objects via XPCNativeWrapper but then lists the following example for adding event listeners:

> anElement.onclick = function() {
>   alert(theMessage);
> };
> 
> anotherElement.addEventListener("click", function() {
>   alert(theMessage);
> });

I didn't verify this but I am mostly certain that the first option (onclick property) won't work. This is listed as the very first limitation of XPCNativeWrapper (https://developer.mozilla.org/en/XPCNativeWrapper#Limitations_of_XPCNativeWrapper) and from all I know it still applies. Even if it didn't - listing this option first is a bad choice given that content scripts usually want to add their event listeners, not replace the existing ones.

Comment 1

[Will Bamberg [:wbamberg]]

I'm fairly sure it does work.

Try: https://builder.addons.mozilla.org/addon/1022722/latest/, and see the interminable discussion in bug 692440. onclick assignment is (I think) one of the things Alex added over XPCNativeWrapper when writing content proxies. But I might be confused.

If so, I don't think this is P1. Perhaps it should list addEventListener first, and suggest that people might prefer to use this.

Comment 2

[Alexandre Poirot [:ochameau]]

Will is right. We do not expose XPCNativeWrapper, but a custom Proxy implementation.
And the first reason why we start using custom Proxies was because of this particular Xraywrapper limitation!! (That, If I remember correctly, has been fixed now!)

If `node.onclick` appears to fail, it is a bug!

So I would say RESOLVED INVALID?

Comment 3

[Wladimir Palant]

Ok, it's invalid then - still, it should recommend addEventListener over onclick IMHO.