Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTMLSelectElement.options #1558

Closed
cevek opened this issue Dec 25, 2014 · 8 comments
Closed

HTMLSelectElement.options #1558

cevek opened this issue Dec 25, 2014 · 8 comments
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue

Comments

@cevek
Copy link

cevek commented Dec 25, 2014

interface HTMLSelectElement {
    options: HTMLSelectElement; // should be HTMLCollection
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Dec 30, 2014
@sophiajt sophiajt added this to the TypeScript 1.5 milestone Jan 12, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.5, TypeScript 1.6 Feb 5, 2015
@mhegazy mhegazy added the Revisit An issue worth coming back to label Mar 24, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.6, TypeScript 1.7 Jul 1, 2015
@robfe
Copy link

robfe commented Jul 21, 2015

Is there any special reason why this one-line fix keeps getting pushed back? https://github.com/Microsoft/TypeScript/blob/master/src/lib/dom.generated.d.ts#L6078

If it's just a matter of priorities I'd love to help out. This one is annoying me since there's no clean way to replace the interface property

@mhegazy mhegazy assigned zhengbli and unassigned mhegazy Jul 21, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2015

@zhengbli can you take a look.

@RyanCavanaugh
Copy link
Member

@robfe it's a prioritization thing. FWIW we're always accepting PRs for lib.d.ts fixes (or anything else that's very low-risk, e.g. wiki edits, comments / formatting improvements, etc).

@robfe
Copy link

robfe commented Jul 23, 2015

I've just noticed that IE 11 itself considers options to be a reference to the select element

var s = document.createElement("select");
console.log(s.options === s); // true in IE 11, false in other browsers

In the older spec options is a HTMLCollection: http://www.w3.org/TR/REC-DOM-Level-1/level-one-html.html#ID-94282980

readonly attribute HTMLCollection options;

But as per http://www.w3.org/TR/html5/forms.html#dom-select-options it's an HTMLOptionsCollection

The options IDL attribute must return an HTMLOptionsCollection rooted at the select node, whose filter matches the elements in the list of options.

where HTMLOptionsCollection is:

interface HTMLOptionsCollection : HTMLCollection {
  // inherits item()
           attribute unsigned long length; // shadows inherited length
  legacycaller HTMLOptionElement? (DOMString name);
  setter creator void (unsigned long index, HTMLOptionElement? option);
  void add((HTMLOptionElement or HTMLOptGroupElement) element, optional (HTMLElement or long)? before = null);
  void remove(long index);
           attribute long selectedIndex;
};

I'm keen to submit a fix for this, but what's your preference @mhegazy @RyanCavanaugh @zhengbli ?

  1. leave it as is (with the interface matching IE11's implementation)?
  2. a one line PR that changes it to HTMLCollection?
  3. a slightly larger PR that adds the HTMLOptionsCollection interface?

@zhengbli
Copy link
Contributor

Confirmed that in Edge the HTMLSelectElement.options is still a reference of itself, the same with IE 11. I suggest to make the type of HTMLSelectElement.options HTMLSelectElement | HTMLCollection to avoid assignability problems.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

that does not help much. now the users who expect it to be HTMLSelectElement do not get that, nor the ones who want it to be HTMLCollection. both groups will have to cast.

@zhengbli
Copy link
Contributor

@mhegazy tested on the latest Edge that HTMLSelectElement.options became HTMLCollection, which is consistent with Chrome and Firefox. It should be OK to use it instead.

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Nov 11, 2015
@mhegazy mhegazy removed the Revisit An issue worth coming back to label Nov 11, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Nov 11, 2015

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

6 participants