Skip to content

Conversation

@annevk
Copy link
Member

@annevk annevk commented Sep 29, 2015

As far as I can tell neither Gecko nor WebKit support legacycaller for
either HTMLFormControlsCollection or HTMLOptionsCollection.

Furthermore, HTMLOptionsCollection’s namedItem() cannot return a
collection since it’s identical to HTMLCollection’s namedItem().

@zcorpan
Copy link
Member

zcorpan commented Sep 29, 2015

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3640 seems like in Blink options['y'] and options.namedItem('y') return a NodeList. But in WebKit it returns an element. What does Edge do?

@annevk
Copy link
Member Author

annevk commented Sep 29, 2015

I get undefined and null in IE11. In IE11 it seems options returns the <select> element which has a namedItem() method, but doesn't work for this example...

@domenic
Copy link
Member

domenic commented Sep 29, 2015

/cc @DigiTec who I know was looking in to this recently for Edge. There are some other issues he was looking in to regarding legacycaller and HTMLAllCollection I think; would be great to get some details there in a separate issue...

@foolip
Copy link
Member

foolip commented Sep 29, 2015

Are these two issues actually related?

  1. The use of legacycaller for HTMLFormControlsCollection and HTMLOptionsCollection
  2. The namedItem() overrides on HTMLFormControlsCollection and HTMLOptionsCollection

Blink doesn't have legacycaller, but does have namedItem() overrides. HTMLFormControlsCollection.namedItem() returns (RadioNodeList or Element)?, while HTMLOptionsCollection.namedItem() returns (NodeList or Element)?.

@annevk
Copy link
Member Author

annevk commented Sep 29, 2015

@foolip well, the specification doesn't have the overrides (except for the non-normative text).

@domenic
Copy link
Member

domenic commented Sep 29, 2015

I'll test Edge when I get to work (home computer is still on Windows 8.1). Too bad remote.modern.ie is not updated.

@foolip
Copy link
Member

foolip commented Sep 29, 2015

@annevk, right, I've added FIXMEs for that, I'm just not sure what the connection is, was this just drive-by cleanup?

@annevk
Copy link
Member Author

annevk commented Sep 29, 2015

@foolip yes, since I had to remove the legacycaller bits from those sections.

@foolip
Copy link
Member

foolip commented Sep 29, 2015

OK then, changes LGTM!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, these are a pretty big mess too. The comment says you inherit the "getter" but how does that even work. And item is shadowed by two overloads, but doesn't have a comment, even though namedItem does. So confusing.

I guess not any different than before though, so it doesn't need to be addressed in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems what Gecko does here is just have a distinct class for this collection. We could do that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm same in Chrome. Edge inherits from HTMLCollection though. Interesting.

@domenic
Copy link
Member

domenic commented Sep 29, 2015

FYI Edge gives the same as IE11 (undefined and "null").

We should also consider removing the microdata API.

@annevk annevk force-pushed the legacycaller branch 2 times, most recently from 7e64271 to 6f73471 Compare September 29, 2015 15:17
@domenic
Copy link
Member

domenic commented Sep 29, 2015

Another thing that would be good to do for this kind of change is at least open an issue on web-platform-tests, or even better, write and submit some tests.

As far as I can tell neither Gecko nor WebKit support legacycaller for
either HTMLFormControlsCollection or HTMLOptionsCollection.

Furthermore, HTMLOptionsCollection’s namedItem() cannot return a
collection since it’s identical to HTMLCollection’s namedItem().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants