Make some improvements to our reporting of insufficient arguments

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
9 months ago
7 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

({dev-doc-complete})

Trunk
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 wontfix, firefox66 fixed)

Details

Attachments

(4 attachments)

Our current error for not enough arguments for JS stuff looks like this:

  TypeError: FUNC requires more than N arguments

and for DOM stuff:

  TypeError: Not enough arguments to FUNC.

I haven't found a way to trigger the "JS stuff" case in Chrome so far, but their DOM case is:

  TypeError: Failed to execute FUNC on OBJTYPE: N arguments required, but only M present.

I'd like to do two things: add reporting of how many args were actually passed to our "JS stuff" and make the DOM use the "JS stuff" mechanism to get the nicer reporting.
Priority: -- → P3
Looks like these uses might predate requireAtLeast existing.
Attachment #9029104 - Flags: review?(nicolas.b.pierron)
We don't want to pay the cost of a function call here in DOM bindings.
Attachment #9029106 - Flags: review?(nicolas.b.pierron)
Comment on attachment 9029104 [details] [diff] [review]
part 1.  Use requireAtLeast more in the JS engine

Review of attachment 9029104 [details] [diff] [review]:
-----------------------------------------------------------------

I had doubts about converting the  non-zero "if (!=)" cases, but they all appears in js.cpp which does not matter spec-wise.
Attachment #9029104 - Flags: review?(nicolas.b.pierron) → review+
Attachment #9029105 - Flags: review?(nicolas.b.pierron) → review+
Attachment #9029106 - Flags: review?(nicolas.b.pierron) → review+
Attachment #9029107 - Flags: review?(nicolas.b.pierron) → review+
Comment on attachment 9029107 [details] [diff] [review]
part 4.  Use CallArgs::requireAtLeast in the DOM

Review of attachment 9029107 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry this got stuck in my queue.
Attachment #9029107 - Flags: review?(kyle) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e37a2cdffa1a
part 1.  Use requireAtLeast more in the JS engine.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/e381de28cdca
part 2.  Have JSMGS_MORE_ARGS_NEEDED say how many args it actually got.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/696e7b4f7535
part 3.  Inline the fast (no error) path of requireAtLeast.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/39b04fe4eae8
part 4.  Use CallArgs::requireAtLeast in the DOM.  r=nbp,qdot
Depends on: 1513213

Looks like Boris already updated the relevant MDN error page. Cheers!

I haven't added a note to the Fx66 rel notes, as I don't think it's really worth it for error message changes.

You need to log in before you can comment on or make changes to this bug.