Add some sweet XML classes.

Add some sweet XML classes.

Committed: https://code.google.com/p/dart/source/detail?r=4645

[Jacob, 2012-02-22 21:55:00]

Top level comments before a more detailed review.
high level view:
Adding an XMLDocument class adds clarity
Adding a publicly visible XMLElement class just adds confusion.  You still need
a separate XMLElementWrappingImpl class but only expose the existing Element
interface.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/Documen...
File client/html/src/DocumentFragment.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/Documen...
client/html/src/DocumentFragment.dart:11: DocumentFragment.xml(String xml);
should we include this method? we don't really want to encourage people to
heavily use XML.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
File client/html/src/XMLDocumentWrappingImplementation.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:22: factory
XMLDocumentWrappingImplementation.xml(String xml) {
Add a TODO warning that this doesn't work on dartium due to DOMParser being
missing.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:30: if
(!xml.toLowerCase().contains('<parsererror') &&
is there a less hacky way to do this?

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:41: case "beforebegin":
return null;
style nit: put returns on the following lines for consistency

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLElem...
File client/html/src/XMLElement.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLElem...
client/html/src/XMLElement.dart:18: XMLElement get offsetParent();
Why is there a user visible XMLElement interface? This seems confusing as its
inverted the normal element interface hierarchy.

[nweiz, 2012-02-23 00:35:06]

There are a couple reasons that XMLElement is separate from Element:

* XMLElement.tag and XMLElement.xml are useful. It would be very confusing to
have constructors for HTML and SVG but not for XML.
* Being able to say "el is XMLElement" is valuable and necessary for consistency
with the rest of the dart:html API.

It's a little weird coming from JS to have XMLElement inherit from Element, but
I think in the abstract it's not that weird... especially since SVGElement,
DocumentFragment, etc. all already inherit from Element.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/Documen...
File client/html/src/DocumentFragment.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/Documen...
client/html/src/DocumentFragment.dart:11: DocumentFragment.xml(String xml);
On 2012/02/22 21:55:00, Jacob wrote:
> should we include this method? we don't really want to encourage people to
> heavily use XML.

Because we allow all Element sub-interfaces to function as Elements, it's very
useful to have something like DocumentFragment that contains multiple elements
without a root. It makes some things much, much easier; see e.g. the
implementation of XMLElementWrappingImplementation#_insertAdjacentNode. We may
not want to encourage XML, but we don't want to make it painful either.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
File client/html/src/XMLDocumentWrappingImplementation.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:22: factory
XMLDocumentWrappingImplementation.xml(String xml) {
On 2012/02/22 21:55:00, Jacob wrote:
> Add a TODO warning that this doesn't work on dartium due to DOMParser being
> missing.

Done (in XMLDocument.dart so it'll show up in the docs).

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:30: if
(!xml.toLowerCase().contains('<parsererror') &&
On 2012/02/22 21:55:00, Jacob wrote:
> is there a less hacky way to do this?

I can't think of one. There doesn't seem to be any way to detect an error other
than the PARSERERROR element.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:41: case "beforebegin":
return null;
On 2012/02/22 21:55:00, Jacob wrote:
> style nit: put returns on the following lines for consistency

Done.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLElem...
File client/html/src/XMLElement.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLElem...
client/html/src/XMLElement.dart:18: XMLElement get offsetParent();
On 2012/02/22 21:55:00, Jacob wrote:
> Why is there a user visible XMLElement interface? This seems confusing as its
> inverted the normal element interface hierarchy.

See my reply to the overall CL comment.

[Jacob, 2012-02-27 22:01:24]

pretty much lgtm. 
One note, can XMLElements ever find their way within an HTML or SVG document? If
so, the types in XMLElement.dart aren't right.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
File client/html/src/XMLDocumentWrappingImplementation.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:30: if
(!xml.toLowerCase().contains('<parsererror') &&
On 2012/02/23 00:35:06, nweiz wrote:
> On 2012/02/22 21:55:00, Jacob wrote:
> > is there a less hacky way to do this?
> 
> I can't think of one. There doesn't seem to be any way to detect an error
other
> than the PARSERERROR element.

one thought, is the parseerror element always the root element and is the
structure very restricted when there is an error?
i'm alright with leaving this if there is a TODO noting it is hacky

[nweiz, 2012-02-27 22:11:58]

Good point about the types. Looks like the DOM allows HTML elements inside XML
elements and vice versa, so I've removed the XMLElement-typed methods.

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
File client/html/src/XMLDocumentWrappingImplementation.dart (right):

https://chromiumcodereview.appspot.com/9430066/diff/1/client/html/src/XMLDocu...
client/html/src/XMLDocumentWrappingImplementation.dart:30: if
(!xml.toLowerCase().contains('<parsererror') &&
On 2012/02/27 22:01:24, Jacob wrote:
> On 2012/02/23 00:35:06, nweiz wrote:
> > On 2012/02/22 21:55:00, Jacob wrote:
> > > is there a less hacky way to do this?
> > 
> > I can't think of one. There doesn't seem to be any way to detect an error
> other
> > than the PARSERERROR element.
> 
> one thought, is the parseerror element always the root element and is the
> structure very restricted when there is an error?
> i'm alright with leaving this if there is a TODO noting it is hacky

The structure varies from browser to browser, so I don't think we should rely on
it. I'll add a TODO.

[Jacob, 2012-02-27 22:34:31]

lgtm