Closed
Bug 793151
Opened 13 years ago
Closed 13 years ago
.length of IDL methods needs to be minimum number of arguments, not maximum
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: ayg, Assigned: Ms2ger)
References
Details
Attachments
(1 file)
|
4.43 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
data:text/html,<!doctype html>
<script>
document.documentElement.textContent = CSSStyleDeclaration.prototype.setProperty.length;
</script>
This is "3" in the September 16 nightly. But the third argument is optional, so it should be "2" per WebIDL. See bug 777061 and discussion there. This is tested by idlharness.js, although the test needs to be synced from upstream to be correct (since WebIDL changed).
IE/WebKit/Opera seem to output 0, but that might be because they make all arguments optional. Or maybe they just always make .length 0.
One reason why the new WebIDL spec might not be as desirable as the old behavior is that it means .length is useless for feature-testing -- any new argument added to an existing function will inevitably be optional. In which case, I don't see much point for the feature to exist, since I can't think of any use other than feature-testing. But I guess matching ES is worthwhile, even if it's pointless.
|
||
Comment 1•13 years ago
|
||
The other option is pointing out to the ES folks that the new behavior is worse and getting ES changed back. ;)
|
Assignee | |
Updated•13 years ago
|
Assignee: nobody → Ms2ger
|
Reporter | |
Comment 2•13 years ago
|
||
The new behavior is arguably worse than the old, but the utility for feature-testing is extremely limited anyway. I'd say .length may as well just be 0 for all functions -- AFAICT from brief testing, this is what IE and WebKit do, and it's certainly simple.
|
|
Assignee | |
Comment 3•13 years ago
|
||
Attachment #671251 -
Flags: review?(bzbarsky)
|
|
||
Comment 4•13 years ago
|
||
Comment on attachment 671251 [details] [diff] [review]
Patch v1
s/retType/_/ because we don't care about it, and r=me.
Yes, we now know more python than we did when we wrote this code. ;)
Attachment #671251 -
Flags: review?(bzbarsky) → review+
|
|
Reporter | |
Comment 5•13 years ago
|
||
Wait, so do we really want this change? Optional params are new in ES6, so why don't we just change ES6 so that they do count toward .length, and change WebIDL to match? I just discussed this on IRC with zcorpan of Opera and he was in favor of changing the spec (although Opera returns 0 here anyway).
Ms2ger, please don't land this until we're sure we don't want the spec to change instead. I just filed a WebIDL bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=19571 And zcorpan mailed public-script-coord: http://lists.w3.org/Archives/Public/public-script-coord/2012OctDec/0039.html
|
|
||
Comment 6•13 years ago
|
||
> so why don't we just change ES6 so that they do count toward .length
Like per comment 1?
I'd be fine with that too, obviously.
|
|
Reporter | |
Comment 7•13 years ago
|
||
(In reply to :Aryeh Gregor from comment #5)
> I just filed a WebIDL bug:
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=19571 And zcorpan mailed
> public-script-coord:
> http://lists.w3.org/Archives/Public/public-script-coord/2012OctDec/0039.html
From the response to those, it looks like the spec won't change, so I guess you may as well land the patch. It's no big loss.
|
|
Assignee | |
Comment 8•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
|
||
Updated•7 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•