405242 - Implement select.options.remove() method [HTML5 HTMLOptionsCollection.remove]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David D. Kilzer (ddk)]

Attachment: Test case

Firefox should support the select.options.remove() method.

MSIE 6/7, Opera 9.24 and (soon) WebKit[1] support it.

[1] http://bugs.webkit.org/show_bug.cgi?id=9683

Firefox and the aforementioned browsers/engines already support the select.options.add() method.

This would apparently have helped to fix part of Bug 278380 when it was originally filed.

Comment 1

[David D. Kilzer (ddk)]

Per Microsoft's documentation (see URL field), the implementation should simply mirror the select.remove() method.  MSDN points to this definition:

http://www.w3.org/TR/2000/WD-DOM-Level-1-20000929/level-one-html.html#ID-33404570

Comment 2

[Nickolay_Ponomarev]

This is described in HTML5 spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#dom-htmloptionscollection-remove
linked from http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-select-options

Comment 3

[Makoto Kato [:m_kato]]

Attachment: patch v1

Comment 4

[Nickolay_Ponomarev]

Makoto: did you forget to request a review here?

Comment 5

[Makoto Kato [:m_kato]]

(In reply to comment #4)
> Makoto: did you forget to request a review here?

mochitest has typo comment, so I will review this after I fix it.

Comment 6

[Makoto Kato [:m_kato]]

Attachment: patch v1.1

IE, WebKit and Opera already implements this method...

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 414806 [details] [diff] [review]
patch v1.1


>+NS_IMETHODIMP
>+nsHTMLOptionCollection::Remove(PRInt32 aIndex)
>+{
>+  return mSelect->Remove(aIndex);
>+}
You should null check mSelect.

Comment 8

[Makoto Kato [:m_kato]]

Attachment: patch v2

Comment 9

[Makoto Kato [:m_kato]]

Attachment: patch v3

Comment 10

[Makoto Kato [:m_kato]]

landed
http://hg.mozilla.org/mozilla-central/rev/6b7741d88c3b