Support SVGElement#elements, #innerHTML, and #outerHTML. Also add a .svg constructor.

Support SVGElement#elements, #innerHTML, and #outerHTML. Also add a .svg constructor.

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

[Jacob, 2011-12-15 21:46:59]

looking very good. a few minor comments.

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
File client/html/src/SVGElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:14: final match =
ElementWrappingImplementation._START_TAG_REGEXP.
move this const to top level instead of within the Element class.

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:52: final elements =
this.elements;
i don't understand why liveness is a concern in this case.

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:74: this.elements =
container.elements[0].elements;
nit:
container.elements.first 
instead of
container.elements[0]

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
File client/tests/client/html/SVGElementTests.dart (right):

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
client/tests/client/html/SVGElementTests.dart:8: var out = new List<String>();
nit:

var --> final where correct

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
client/tests/client/html/SVGElementTests.dart:44: (e) => e == 'SVG had 0
top-level elements but 1 expected');
we should just check that it throws, not enforce a specific error message.
We should give the errors a reasonable type so we can just check the type.

[nweiz, 2011-12-15 23:02:01]

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
File client/html/src/SVGElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:14: final match =
ElementWrappingImplementation._START_TAG_REGEXP.
On 2011/12/15 21:46:59, Jacob wrote:
> move this const to top level instead of within the Element class.

Done.

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:52: final elements =
this.elements;
On 2011/12/15 21:46:59, Jacob wrote:
> i don't understand why liveness is a concern in this case.

I guess I just copied this from ElementWrappingImplementation without looking at
it too deeply. Fixed here and there as well.

http://codereview.chromium.org/8965006/diff/1/client/html/src/SVGElementWrapp...
client/html/src/SVGElementWrappingImplementation.dart:74: this.elements =
container.elements[0].elements;
On 2011/12/15 21:46:59, Jacob wrote:
> nit:
> container.elements.first 
> instead of
> container.elements[0]

Done.

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
File client/tests/client/html/SVGElementTests.dart (right):

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
client/tests/client/html/SVGElementTests.dart:8: var out = new List<String>();
On 2011/12/15 21:46:59, Jacob wrote:
> nit:
> 
> var --> final where correct

Done.

http://codereview.chromium.org/8965006/diff/1/client/tests/client/html/SVGEle...
client/tests/client/html/SVGElementTests.dart:44: (e) => e == 'SVG had 0
top-level elements but 1 expected');
On 2011/12/15 21:46:59, Jacob wrote:
> we should just check that it throws, not enforce a specific error message.
> We should give the errors a reasonable type so we can just check the type.

I've changed this and Element.html to throw IllegalArgumentExceptions.

[Jacob, 2012-01-04 22:54:31]

lgtm

[sra1, 2012-01-05 04:35:27]

http://codereview.chromium.org/8965006/diff/2002/client/html/src/SVGElementWr...
File client/html/src/SVGElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8965006/diff/2002/client/html/src/SVGElementWr...
client/html/src/SVGElementWrappingImplementation.dart:62:
container.elements.addAll(this.clone(true).elements);
This breaks the build because the static type returned by clone is Node.

dart://html/html/htmlimpl.dart:24074: "elements" is not a member of Node
 24073:     final container = new Element.tag("div");
 24074:     container.elements.addAll(this.clone(true).elements);
                                                       ~~~~~~~~

Add a .dynamic

[Jacob, 2012-01-05 16:39:48]

http://codereview.chromium.org/8965006/diff/2002/client/html/src/SVGElementWr...
File client/html/src/SVGElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8965006/diff/2002/client/html/src/SVGElementWr...
client/html/src/SVGElementWrappingImplementation.dart:62:
container.elements.addAll(this.clone(true).elements);
On 2012/01/05 04:35:27, sra1 wrote:
> This breaks the build because the static type returned by clone is Node.
> 
> dart://html/html/htmlimpl.dart:24074: "elements" is not a member of Node
>  24073:     final container = new Element.tag("div");
>  24074:     container.elements.addAll(this.clone(true).elements);
>                                                        ~~~~~~~~
> 
> Add a .dynamic

The better fix is to define .clone on Element as well as Node. The version for
Element should have a return type of Element rather than Node.