Note: There are a few cases of duplicates in user autocompletion which are being worked on.

NS_ERROR_XPC_NOT_ENOUGH_ARGS on rumpetroll.com (investigate if we should have optional useCapture parameter)

RESOLVED FIXED in mozilla6

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: jdm, Assigned: smaug)

Tracking

({dev-doc-complete})

unspecified
mozilla6
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Whiteboard: [parity-webkit][parity-opera][parity-IE9?])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
STR:
1. Visit rumpetroll.com

Expected:
Swimming around in a fun websocket-based game

Actual:
The error appears in the console, no game is present.

This works fine in chrome.
(Reporter)

Comment 1

7 years ago
This is the failing line in main.js:

>document.getElementById('authorize-user-button').addEventListener('click', function(e) { 
>		app.authorize(null,null);
>		authWindow = window.open("auth.html","","width=950,height=460,menubar=no,toolbar=no,location=no,directories=no,status=no,scrollbars=yes,resizable=yes')")
>		return false;
>	});
(Reporter)

Comment 2

7 years ago
This error is occurring because |addEventListener('click', function(e) {})| throws.  CCing people who might know whether this is correct behaviour or not.
Component: General → DOM: Core & HTML
QA Contact: general → general

Comment 3

7 years ago
It's correct behavior per DOM 2 Events, and this site is assuming Webkit's behavior which doesn't follow that spec, really.

There have been proposals to change it for Web DOM Core; I don't believe we have a bug on making the third arg optional so far.
Still required in DOM3 Events: <http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-EventTarget>. I filed <http://www.w3.org/Bugs/Public/show_bug.cgi?id=11358>.
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 5

7 years ago
Created attachment 492203 [details] [diff] [review]
optional capture param

I pushed this to tryserver
(Assignee)

Updated

7 years ago
Summary: NS_ERROR_XPC_NOT_ENOUGH_ARGS on rumpetroll.com → NS_ERROR_XPC_NOT_ENOUGH_ARGS on rumpetroll.com (investigate if we should have optional useCapture parameter)
(Assignee)

Comment 6

7 years ago
Apparently that kind of simple patch breaks quite a few tests.

Comment 7

7 years ago
Well, at least the ones in the official DOM test suites, right?
http://mxr.mozilla.org/mozilla-central/source/dom/src/threads/nsDOMWorkerMessageHandler.cpp#339

342 NS_IMETHODIMP
343 nsDOMWorkerMessageHandler::AddEventListener(const nsAString& aType,
344                                             nsIDOMEventListener* aListener,
345                                             PRBool aUseCapture,
346                                             PRBool aWantsUntrusted,
347                                             PRUint8 optional_argc)
348 {
349   // We don't support aWantsUntrusted yet.
350   NS_ENSURE_TRUE(optional_argc == 0, NS_ERROR_NOT_IMPLEMENTED);

This isn't going to fly anymore...
Blocks: 616252

Updated

7 years ago
Whiteboard: [parity-webkit][parity-opera]
(Assignee)

Comment 9

7 years ago
We may want to fix this for FF4.
If I understood Travis (from MS) correctly, IE9 may make the useCapture
optional too.
Assignee: nobody → Olli.Pettay

Updated

7 years ago
blocking2.0: --- → ?
Whiteboard: [parity-webkit][parity-opera] → [parity-webkit][parity-opera][parity-IE9?]
As mentioned in http://www.w3.org/mid/20101124025438.GA14523@wok.mcc.id.au we came to the conclusion that Web IDL should allow too few arguments, and coerce them from undefined, (and allow too many, and ignore the extra arguments) as the most JavaScript-y way to handle mismatched arity.  Nobody has spoken up against this proposal on the list yet.  (The change is yet to have been made to the spec.)

The result of this is that it wouldn't matter if useCapture is optional or not -- omitting it would require the missing argument to be treated as undefined and thus be converted to false.
(Assignee)

Comment 11

7 years ago
Not sure if this really needs to *block*.
Agreed, we can get to this later.
blocking2.0: ? → .x

Comment 13

6 years ago
Is there a technical or spec holdup to fixing this?

We've already made the second parameter of window.getComputedStyle to be optional.
(Assignee)

Comment 14

6 years ago
(In reply to comment #13)
> Is there a technical or spec holdup to fixing this?
No. I just need to find time to fix this.
Also note that Web DOM Core and DOM 3 Events have changed to make the argument optional:

  http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#eventtarget
  http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-EventTarget-addEventListener

Comment 16

6 years ago
Olli, is there anything to do here other than adding "optional" to the idl and writing tests?
(Assignee)

Comment 17

6 years ago
I did try that [optional] approach and something broke.
The reason is, IIRC, that we already have one optional parameter, so need to
update optional_argc handling.

Comment 18

6 years ago
(In reply to comment #17)
> I did try that [optional] approach and something broke.
> The reason is, IIRC, that we already have one optional parameter, so need to
> update optional_argc handling.

This problem only affects the nsIDOMNSEventTarget interface, which web content doesn't use, AIUI. Could we could first land a patch that makes useCapture optional only for nsIDOMEventTarget?
(Assignee)

Comment 19

6 years ago
Web content does use addEventListener from nsIDOMNSEventTarget.
addEventListener in nsIDOMEventTarget is [noscript]
(Assignee)

Comment 20

6 years ago
Created attachment 527936 [details] [diff] [review]
patch

Uploaded to tryserver
Attachment #492203 - Attachment is obsolete: true
Attachment #527936 - Flags: review?(jonas)

Updated

6 years ago
Duplicate of this bug: 646687
Comment on attachment 527936 [details] [diff] [review]
patch

Maybe also test that

x.addEventListener("foo", l);
x.removeEventListener("foo, l, false);

removes the event listener?
Attachment #527936 - Flags: review?(jonas) → review+

Updated

6 years ago
Depends on: 654770
(Assignee)

Comment 23

6 years ago
http://hg.mozilla.org/mozilla-central/rev/f0ad024522bb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Keywords: dev-doc-needed
Target Milestone: --- → mozilla6
Documentation updated:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMEventTarget
https://developer.mozilla.org/en/DOM/element.addEventListener

And mentioned on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.