Last Comment Bug 182279 - select.add() should have the second parameter optional
: select.add() should have the second parameter optional
Status: VERIFIED FIXED
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- minor with 2 votes (vote)
: mozilla7
Assigned To: Makoto Kato [:m_kato]
:
: Andrew Overholt [:overholt]
Mentors:
http://www.whatwg.org/html/#the-selec...
: 140427 194121 378479 387314 (view as bug list)
Depends on:
Blocks: html5forms
  Show dependency treegraph
 
Reported: 2002-11-27 11:13 PST by Mason
Modified: 2011-09-14 06:21 PDT (History)
16 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test Case (448 bytes, text/html)
2002-11-27 11:17 PST, Mason
no flags Details
fix (3.74 KB, patch)
2011-06-20 02:35 PDT, Makoto Kato [:m_kato]
bugs: review+
mounir: checkin+
Details | Diff | Splinter Review

Description Mason 2002-11-27 11:13:10 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2) Gecko/20021126
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2) Gecko/20021126

IE and Moz1.2b- supported selectTag.add( option ) (syntax, not exact code). ex:
<script>
var select = document.getElementById( "MYSELECT" );
var opt = document.createElement( "OPTION" );
opt.value = "TEST";
opt.text  = "TEST TEXT";
try
{
select.add( opt );
}
catch( exc )
{
  alert( exc );
}
</script>

This now fails

Reproducible: Always

Steps to Reproduce:
See Details
select.add( opt )
Actual Results:  
select.add takes 2 arguments in XPCOM.  Both are HTMLElements.  I attempted to
pass the option as both parameters, but nothing occurred.  I couldn't find any
XPCOM documentation for what they should be.

Expected Results:  
The select element should add an option at the end of the list
Comment 1 Mason 2002-11-27 11:17:09 PST
Created attachment 107607 [details]
Test Case
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2002-11-27 12:26:38 PST
http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-94282980:

add
    Add a new element to the collection of OPTION elements for this SELECT. This
method is the equivalent of the appendChild method of the Node interface if the
before parameter is null. It is equivalent to the insertBefore method on the
parent of before in all other cases. This method may have no effect if the new
element is not an OPTION or an OPTGROUP.
    Parameters

    element of type HTMLElement
        The element to add.
    before of type HTMLElement
        The element to insert before, or null for the tail of the list.

Yes, the MS version of add() is not compatible with the W3C DOM.  That's too bad.
Comment 3 Jesse Ruderman 2007-04-23 13:25:06 PDT
*** Bug 140427 has been marked as a duplicate of this bug. ***
Comment 4 Jesse Ruderman 2007-04-23 13:27:45 PDT
*** Bug 194121 has been marked as a duplicate of this bug. ***
Comment 5 Jesse Ruderman 2007-04-23 13:27:57 PDT
*** Bug 378479 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Paz 2007-04-23 13:34:14 PDT
(In reply to comment #2)
> http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-94282980:
> 
> add
>     Add a new element to the collection of OPTION elements for this SELECT. This
> method is the equivalent of the appendChild method of the Node interface if the
> before parameter is null. It is equivalent to the insertBefore method on the
> parent of before in all other cases. This method may have no effect if the new
> element is not an OPTION or an OPTGROUP.
>     Parameters
> 
>     element of type HTMLElement
>         The element to add.
>     before of type HTMLElement
>         The element to insert before, or null for the tail of the list.
> 
> Yes, the MS version of add() is not compatible with the W3C DOM.  That's too bad.

It is compatible. It takes into account that JavaScript will pass "undefined" for any argument that is not explicitly set. The W3C DOM specifies that "null" can be passed to the second variable, and "undefined" is effectively "null" in JavaScript.

select.add(opt) should operate the same as select.add(opt,undefined). When you explicitly set the second variable to "undefined", it will work as intended in both IE and Firefox. 
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2007-04-23 13:38:04 PDT
> and "undefined" is effectively "null" in JavaScript.

No, it's not....  And in any case, native and host functions are treated differently both in JavaScript and the ECMAScript specification.  Handling of fewer args than arity is one of the differences in this case.

> select.add(opt) should operate the same as select.add(opt,undefined).

If "add" were a native ECMAScript function, yes.  It's not.  It's a host function.
Comment 8 Jonathan Paz 2007-04-23 13:40:22 PDT
Ah. I see.

Thank you very much for that clarification.
Comment 9 Karl Rosenberger 2007-07-16 11:42:27 PDT
I'm having similar issues getting this to work with select.add(opt,x).

It's not inserting the option element into the select list, nor is it generating an error.  select.add(opt) doesn't work either.

Works fine in IE. :-(

I'd much prefer to use select.add than working with nodes and appendChild/insertBefore syntax.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2007-07-16 11:49:42 PDT
Karl, that sounds like a different bug. Please file it and attach the script that's not working.  cc me on it, ok?
Comment 11 Karl Rosenberger 2007-07-16 13:22:39 PDT
Oops.  My bad.  Just realized that I was using the non-standard select.add(opt,select.selectedIndex) rather than select.add(opt,select[select.selectedIndex]).
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2010-12-04 03:06:10 PST
*** Bug 387314 has been marked as a duplicate of this bug. ***
Comment 13 Mounir Lamouri (:mounir) 2010-12-14 13:51:07 PST
If the second parameter is omitted, the option should be added at the end of the list.
Comment 14 Makoto Kato [:m_kato] 2011-06-20 02:35:29 PDT
Created attachment 540412 [details] [diff] [review]
fix
Comment 15 Mounir Lamouri (:mounir) 2011-06-20 04:37:34 PDT
I thought that when [optional] was used, it was required to add a PRUint8 parameter or is that only needed when using an optional argument with no sane default value like an integer?
Comment 16 Mounir Lamouri (:mounir) 2011-06-20 04:38:51 PDT
Comment on attachment 540412 [details] [diff] [review]
fix

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

::: content/html/content/test/test_bug182279.html
@@ +28,5 @@
> +sel.add(opt2, opt1);
> +sel.add(opt3);
> +is(sel[0].value, 2, "1st item should be 2");
> +is(sel[1].value, 1, "2nd item should be 1");
> +is(sel[2].value, 3, "3rd item shoulb be 3");

Maybe it would be better to compare sel[x] with optX instead of the values.
Comment 17 Olli Pettay [:smaug] 2011-06-20 04:47:40 PDT
[optional] doesn't generate C++ which has PRUint8 parameter.
[optional_argc] does.

See http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIXMLHttpRequest.idl#261 vs http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIXMLHttpRequest.idl#231
If you have several optional parameters, and the default isn't something you
want, then you should use [optional_argc].

In this case [optional] is enough.
Comment 18 Makoto Kato [:m_kato] 2011-06-21 04:09:11 PDT
(In reply to comment #16)
 
> Maybe it would be better to compare sel[x] with optX instead of the values.

I will modify it when landing this.
Comment 20 Mounir Lamouri (:mounir) 2011-06-22 01:23:59 PDT
Makoto, by any chance, would you like to add the select.add(option, position); method? I think we don't have it.
Comment 21 Makoto Kato [:m_kato] 2011-06-22 01:34:46 PDT
(In reply to comment #20)
> Makoto, by any chance, would you like to add the select.add(option,
> position); method? I think we don't have it.

I believe that xpidl doesn't support same method name that the parameters are different.  It causes conflict name error.  So we should fix xpidl (and  xpconnect?) for current WebIDL spec.

Using jsval as parameter, it may be able to emulate it.  But it becomes too complex codes and I don't test it.

I will file some bugs for these.
Comment 22 Makoto Kato [:m_kato] 2011-06-22 01:35:05 PDT
Also, HTMLOptionsCollection.add() has same problem.
Comment 23 Mounir Lamouri (:mounir) 2011-06-22 01:42:55 PDT
(In reply to comment #21)
> (In reply to comment #20)
> > Makoto, by any chance, would you like to add the select.add(option,
> > position); method? I think we don't have it.
> 
> I believe that xpidl doesn't support same method name that the parameters
> are different.  It causes conflict name error.  So we should fix xpidl (and 
> xpconnect?) for current WebIDL spec.

Maybe this will be fixed with pyxpidl (bug 660861). I mean, if xpconnect isn't blocking us (and I don't know if it is).
Comment 24 Makoto Kato [:m_kato] 2011-06-22 02:02:03 PDT
(In reply to comment #23)

> Maybe this will be fixed with pyxpidl (bug 660861). I mean, if xpconnect
> isn't blocking us (and I don't know if it is).

Should I file a new bug?  Although I test this now, pyidl throws "name 'add' specified twice".

When using xpidl, it throws "`add' conflicts".
Comment 25 Makoto Kato [:m_kato] 2011-06-22 02:11:47 PDT
file bug 666199 for comment #21 issue
Comment 26 Makoto Kato [:m_kato] 2011-06-22 02:18:40 PDT
(In reply to comment #20)
> Makoto, by any chance, would you like to add the select.add(option,
> position); method? I think we don't have it.

filed bug 666200.
Comment 27 Olli Pettay [:smaug] 2011-06-22 05:39:33 PDT
One can always nsIVariant as a the type of the parameter.
Comment 28 Olli Pettay [:smaug] 2011-06-22 05:39:57 PDT
One can always use...
Comment 29 Matt Brubeck (:mbrubeck) 2011-06-22 10:22:21 PDT
http://hg.mozilla.org/mozilla-central/rev/ff25dce5b886
Comment 30 Boris Zbarsky [:bz] (still a bit busy) 2011-06-22 10:39:03 PDT
> I mean, if xpconnect isn't blocking us

XPConnect will by default map one JS function to one C++ function.

So to do overloads either you need to declare as nsIVariant or jsval and do your own argument handling or you need to write a custom quickstub (and do your own argument handling).

We hope to automate the latter at some point, and then the IDL will be able to allow declaring overloads...
Comment 31 Eric Shepherd [:sheppy] 2011-06-23 06:40:13 PDT
Documentation updated by Trevor.
Comment 32 Ioana (away) 2011-09-14 06:21:16 PDT
Both content/html/content/test/test_bug182279.html and the test case uploaded by the  reporter were run on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
+ Aurora (Fx 8.0a2) & Nightly (Fx 9.0a1).

Both tests passed.

STR (for the test case uploaded by the bug reporter):
1. Load the test case in a Fx tab.
2. Click on the AttemptAdd button a few times.
Actual Results:
 For every click on the AttemptAdd button, the "Add Test" element is added at the end of the Select elements list.

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