Last Comment Bug 720385 - Implement index property on <option> in <datalist>
: Implement index property on <option> in <datalist>
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla13
Assigned To: Mounir Lamouri (:mounir)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 555840
  Show dependency treegraph
Reported: 2012-01-23 09:07 PST by Codacoder
Modified: 2012-04-24 15:39 PDT (History)
7 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase comparing <select> and <datalist> behaviour (1.75 KB, text/html)
2012-02-15 15:17 PST, Matthew N. [:MattN] (PM if requests are blocking you)
no flags Details
Patch v1 (5.31 KB, patch)
2012-02-16 10:53 PST, Mounir Lamouri (:mounir)
Ms2ger: review+
mounir: checkin+
Details | Diff | Splinter Review

Description User image Codacoder 2012-01-23 09:07:19 PST
User Agent: Mozilla/5.0 (Windows NT 6.0; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

Created a datalist with simple set of options.  Each option has an id.

Actual results:

opt = opts["id-of-opt"]  
opt.index <-- returns -1
opt = opts[0] 
opt.index <-- returns -1

Expected results:

option.index should work as specified - return index of option in collection.
Comment 1 User image Matthew N. [:MattN] (PM if requests are blocking you) 2012-02-15 00:14:12 PST
I can confirm that the index property only returns -1 when the <option>s are in a <datalist> although it works fine when they are instead in a <select>.

"An option element's index is the number of option element that are in the same list of options but that come before it in tree order. If the option element is not in a list of options, then the option element's index is zero." [1]
Comment 2 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-15 04:32:28 PST
"list of options" is only defined for select elements, not for datalists, so we should return 0.
Comment 3 User image Matthew N. [:MattN] (PM if requests are blocking you) 2012-02-15 15:17:18 PST
Created attachment 597578 [details]
Testcase comparing <select> and <datalist> behaviour

It may be worth proposing a spec change to have "list of options" also defined for <datalist> but currently Opera, & Webkit align with the spec.
Comment 4 User image Matthew N. [:MattN] (PM if requests are blocking you) 2012-02-15 15:22:37 PST
IE 10 PP also returns 0 for the index property of an <option> in a <datalist>.
Comment 5 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-16 01:12:09 PST
Code is here:

and needs to be updated to match the specification at
Comment 6 User image Codacoder 2012-02-16 04:50:50 PST
Yep you guys a correct - my bad.  

I just wonder what the whatwg were smoking when they came up with a datalist element (note "list") that contains a "list of options" that somehow does not qualify as a list-of-options as per the spec:

"If the option element is not in a list of options, then the option element's index is zero."

And I'm told by one of them "It's too late to change it, in any case; it's already shipped in browsers."  I'm further told it's only intended for use by the list="" attribute on other controls.  I guess it must be my fault for trying to use a datalist as a, uh, "list" of, you know, "data".

Anyway, yes, zero it should be.
Comment 7 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-16 05:03:06 PST
(In reply to Russ from comment #6)

Unhelpful. if you want the spec to be changed, there are numerous ways to submit feedback (see This bug, however, is not one of those ways.
Comment 8 User image Codacoder 2012-02-16 05:22:22 PST
Unhelpful? To whom?  What???  The original report is wrong, agreed.  It's still a bug (as per spec - should return zero, mentioned in my comment, at the top and the bottom).  Rest of my comment was sharing "where I'd been".

So, let's be helpful:

When an option is a member of a list of options that make up the list of options within a datalist element the option is not considered to be a member of a [list of options] as defined by the spec.
Comment 9 User image Mounir Lamouri (:mounir) 2012-02-16 07:26:13 PST
What would that change if you had .index returning a value other than 0? Unless there is a real use case, I think we should just comply with the specs: it will be hard to have all browsers to move to something else for no big enough reason.
Comment 10 User image Codacoder 2012-02-16 07:31:50 PST
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #9)
"List" implies "order".  Order implies position.  Position (index of) should be discoverable.  An index of zero is meaningless.  What we have is a bag, or a set, not a list.

Yes, we should comply with the spec.  No one is suggesting otherwise.
Comment 11 User image Mounir Lamouri (:mounir) 2012-02-16 10:53:28 PST
Created attachment 597895 [details] [diff] [review]
Patch v1

Ms2ger, what about that patch? :)
Comment 12 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-16 15:13:43 PST
Looks fine on first sight, but I want to cross-check more carefully with the spec. I hope to get to it tomorrow.
Comment 13 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-17 06:02:01 PST
Comment on attachment 597895 [details] [diff] [review]
Patch v1

Review of attachment 597895 [details] [diff] [review]:

r=me with those changes

::: content/html/content/src/nsHTMLOptionElement.cpp
@@ +208,4 @@
> +  // Get the options from the select object.
> +  nsCOMPtr<nsIDOMHTMLOptionsCollection> options;
> +  selectElement->GetOptions(getter_AddRefs(options));

nsHTMLOptionCollection* options = selectElement->GetOptions();

@@ +229,2 @@
>      }
>    }

How about

PRInt32 index = 0;
if (NS_SUCCEEDED(options->GetOptionIndex(this, 0, true, &index)) {
  *aIndex = index;
Comment 14 User image Mounir Lamouri (:mounir) 2012-02-19 15:38:38 PST
(In reply to Ms2ger from comment #13)
> PRInt32 index = 0;
> if (NS_SUCCEEDED(options->GetOptionIndex(this, 0, true, &index)) {
>   *aIndex = index;
> }

Given that index shouldn't be set when there is a failure and a failure shouldn't happen, I think we can do:
return options->GetOptionIndex(this, 0, true, aIndex);

Ok with that Ms2ger?
Comment 15 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-20 08:36:50 PST
That sounds a little scary, but I can live with it if you add those givens to the documentation for GetOptionIndex.
Comment 16 User image :Ms2ger (⌚ UTC+1/+2) 2012-02-24 13:05:58 PST
Oh, and you can invent a better commit message ;)
Comment 17 User image Marco Bonardo [::mak] 2012-02-25 02:22:40 PST

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