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

RESOLVED FIXED in mozilla1.9.3a2

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: David D. Kilzer (ddk), Assigned: m_kato)

Tracking

({html5, testcase})

Trunk
mozilla1.9.3a2
html5, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
Created attachment 290047 [details]
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.
(Reporter)

Comment 1

10 years ago
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

8 years ago
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
Component: DOM: Mozilla Extensions → DOM: Core & HTML
Keywords: html5
Summary: Implement select.options.remove() method → Implement select.options.remove() method [HTML5 HTMLOptionsCollection.remove]
Version: 1.8 Branch → Trunk
(Assignee)

Updated

8 years ago
(Assignee)

Comment 3

8 years ago
Created attachment 414218 [details] [diff] [review]
patch v1

Comment 4

8 years ago
Makoto: did you forget to request a review here?
(Assignee)

Comment 5

8 years ago
(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.
(Assignee)

Comment 6

8 years ago
Created attachment 414806 [details] [diff] [review]
patch v1.1

IE, WebKit and Opera already implements this method...
Attachment #414218 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Attachment #414218 - Flags: review?(jst)
(Assignee)

Updated

8 years ago
Attachment #414218 - Flags: review?(jst)
(Assignee)

Updated

8 years ago
Attachment #414806 - Flags: review?(jst)

Comment 7

8 years ago
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.
(Assignee)

Comment 8

8 years ago
Created attachment 415069 [details] [diff] [review]
patch v2
Attachment #414806 - Attachment is obsolete: true
Attachment #414806 - Flags: review?(jst)
(Assignee)

Updated

8 years ago
Attachment #415069 - Flags: review?(jst)
(Assignee)

Comment 9

8 years ago
Created attachment 425732 [details] [diff] [review]
patch v3
Attachment #415069 - Attachment is obsolete: true
Attachment #415069 - Flags: review?(jst)
(Assignee)

Updated

8 years ago
Attachment #425732 - Flags: review?(Olli.Pettay)

Updated

8 years ago
Attachment #425732 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Updated

8 years ago
Attachment #425732 - Flags: superreview?(jst)

Updated

8 years ago
Attachment #425732 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 10

8 years ago
landed
http://hg.mozilla.org/mozilla-central/rev/6b7741d88c3b
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
You need to log in before you can comment on or make changes to this bug.