Skip to content

NodeList.filter and NodeList.getRange does not work as advertised #5369

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.

Sign up for GitHub

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

Jump to bottom
Closed
jmesserly opened this issue Sep 21, 2012 · 5 comments
Closed

NodeList.filter and NodeList.getRange does not work as advertised #5369

jmesserly opened this issue Sep 21, 2012 · 5 comments
Assignees
@blois
Labels
web-libraries Issues impacting dart:html, etc., libraries
Milestone
M2

Comments

@jmesserly
Copy link

NodeList.filter returns a NodeList, but it doesn't behave like a normal NodeList. If you mutate this list, the node is not added to the document. Also, no error is reported. Contrast with .elements, which returns a frozen list.
http://api.dartlang.org/docs/continuous/dart_html/NodeList.html

one of these solutions would be better:

  • declare return type as List<Node>, and return mutable list. Essentially like it does now, but the type better aligns with user expectations.
  • return immutable NodeList, like Element.elements does

here's a way to reproduce this:

<!doctype html>
<html>
<head></head>
<body>
<div class="awesome">Hello there</div>
<script type="application/dart">

import('dart:html');

main() {
  // Yeah, you could do this other ways, like with a query :)
  var nodes = document.body.nodes;
  // This won't be added and won't produce an error
  nodes.filter((n) => true).add(new DivElement()..text = ' this no worky!');
  nodes.add(new DivElement()..text = ' but this does!');
}
</script>
<script type="text/javascript">navigator.webkitStartDart();</script>
</body>
</html>
@jmesserly
Copy link
Author

cleared milestone so y'all can triage. I don't see this as being very important, but wanted to file a bug to track

Removed this from the M2 milestone.

@blois
Copy link

blois commented Oct 15, 2012

Set owner to @blois.
Added this to the M2 milestone.

@blois
Copy link

blois commented Oct 16, 2012

Issue #1183 has been merged into this issue.

cc @vsmenon.
cc @rakudrama.

@jmesserly
Copy link
Author

I think NodeList is gone now. That helps a lot with user expectations. Although maybe filter should return Iterable?

@blois
Copy link

blois commented Oct 16, 2012

Currently Node.nodes.filter and getRange match the behavior elsewhere, I'm following up with having filter (and Map.getKeys and map.getValues) return Iterable rather than Collection.

Added Fixed label.

@jmesserly jmesserly added Type-Defect web-libraries Issues impacting dart:html, etc., libraries labels Oct 16, 2012
@jmesserly jmesserly added this to the M2 milestone Oct 16, 2012
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blois blois

Labels
web-libraries Issues impacting dart:html, etc., libraries
Projects
None yet
Milestone
M2
Development

No branches or pull requests
2 participants
@jmesserly @blois