Adding Element.AddHTML and Element.addText methods.

Adding Element.AddHTML and Element.addText methods.

These are to make it easier to append HTML and text to existing DOM nodes.


BUG=3685
Committed: https://code.google.com/p/dart/source/detail?r=11249

[vsm, 2012-08-22 18:22:27]

Looking pretty good.  Some minor comments.

https://chromiumcodereview.appspot.com/10854208/diff/2001/lib/html/doc/interf...
File lib/html/doc/interface/Element.dartdoc (right):

https://chromiumcodereview.appspot.com/10854208/diff/2001/lib/html/doc/interf...
lib/html/doc/interface/Element.dartdoc:88: void addText(String text) {
Can you add a brief comment on each of these?

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
File tests/html/element_add_test.dart (right):

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:1: // Copyright (c) 2011, the Dart project
authors.  Please see the AUTHORS file
s/2011/2012/

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:5: // Put universally passing event
constructors in this file.
This comment doesn't seem to match this test.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:26: Expect.equals(1, el.elements.length);
Sorry this isn't clear anywhere, but the old Expect class is being deprecated in
favor of the new style.  This should be:
  expect(e1.elements.length, equals(1))
and similar below.  "expect" is a top-level method in unittest.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:28: Expect.isTrue(section is
TableSectionElement);
expect(section is TableSectionElement);

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:30: Expect.isTrue(section.elements[0] is
TableRowElement);
How about testing a second add to ensure that it's going to the end?

[blois, 2012-08-22 20:06:13]

https://chromiumcodereview.appspot.com/10854208/diff/2001/lib/html/doc/interf...
File lib/html/doc/interface/Element.dartdoc (right):

https://chromiumcodereview.appspot.com/10854208/diff/2001/lib/html/doc/interf...
lib/html/doc/interface/Element.dartdoc:88: void addText(String text) {
On 2012/08/22 18:22:27, vsm wrote:
> Can you add a brief comment on each of these?

Done.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
File tests/html/element_add_test.dart (right):

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:1: // Copyright (c) 2011, the Dart project
authors.  Please see the AUTHORS file
On 2012/08/22 18:22:27, vsm wrote:
> s/2011/2012/

Done.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:5: // Put universally passing event
constructors in this file.
On 2012/08/22 18:22:27, vsm wrote:
> This comment doesn't seem to match this test.

D'oh. Copy paste :(

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:26: Expect.equals(1, el.elements.length);
On 2012/08/22 18:22:27, vsm wrote:
> Sorry this isn't clear anywhere, but the old Expect class is being deprecated
in
> favor of the new style.  This should be:
>   expect(e1.elements.length, equals(1))
> and similar below.  "expect" is a top-level method in unittest.

Done.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:28: Expect.isTrue(section is
TableSectionElement);
On 2012/08/22 18:22:27, vsm wrote:
> expect(section is TableSectionElement);

Done.

https://chromiumcodereview.appspot.com/10854208/diff/2001/tests/html/element_...
tests/html/element_add_test.dart:30: Expect.isTrue(section.elements[0] is
TableRowElement);
On 2012/08/22 18:22:27, vsm wrote:
> How about testing a second add to ensure that it's going to the end?

Done.

[vsm, 2012-08-22 20:09:34]

lgtm, but please add the copyright back at the top of the test before landing.

https://chromiumcodereview.appspot.com/10854208/diff/7001/tests/html/element_...
File tests/html/element_add_test.dart (right):

https://chromiumcodereview.appspot.com/10854208/diff/7001/tests/html/element_...
tests/html/element_add_test.dart:1: #library('ElementAddTest');
Please add back the copyright (with 2012).

[blois, 2012-08-22 20:40:59]

https://chromiumcodereview.appspot.com/10854208/diff/7001/tests/html/element_...
File tests/html/element_add_test.dart (right):

https://chromiumcodereview.appspot.com/10854208/diff/7001/tests/html/element_...
tests/html/element_add_test.dart:1: #library('ElementAddTest');
On 2012/08/22 20:09:34, vsm wrote:
> Please add back the copyright (with 2012).

Done.

[vsm, 2012-08-22 20:42:09]

thanks, still lgtm!

[blois, 2012-08-23 17:48:09]

On 2012/08/22 20:42:09, vsm wrote:
> thanks, still lgtm!

I realized that the tests were failing on FF because insertAdjacent* wasn't
supported (see bug #3600). I decided to roll the fix for that into this CL just
to get this fully working in one submit.

[vsm, 2012-08-23 18:02:36]

lgtm