HTMLTableSectionElement.insertRow()'s argument default value should be -1

Source/core/html/HTMLTableSectionElement.cpp

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2010 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 37 matching lines @@
     return adoptRef(new HTMLTableSectionElement(tagName, document));
 }
 
 const StylePropertySet* HTMLTableSectionElement::additionalPresentationAttributeStyle()
 {
     if (HTMLTableElement* table = findParentTable())
         return table->additionalGroupStyle(true);
     return 0;
 }
 
+PassRefPtr<HTMLElement> HTMLTableSectionElement::insertRow(ExceptionState& exceptionState)
+{
+    // The default 'index' argument value is -1.
+    return insertRow(-1, exceptionState);
+}
+
 // these functions are rather slow, since we need to get the row at
 // the index... but they aren't used during usual HTML parsing anyway
 PassRefPtr<HTMLElement> HTMLTableSectionElement::insertRow(int index, ExceptionState& exceptionState)
 {
-    RefPtr<HTMLTableRowElement> row;
     RefPtr<HTMLCollection> children = rows();
-    int numRows = children ? (int)children->length() : 0;
+    int numRows = children ? static_cast<int>(children->length()) : 0;
     if (index < -1 || index > numRows) {

[eseidel, 2014/05/05 15:33:42]

Lame that indicies don't work like python here. :)  -2 should be totally valid
IMO. :)

         exceptionState.throwDOMException(IndexSizeError, "The provided index (" + String::number(index) + " is outside the range [-1, " + String::number(numRows) + "].");
+        return nullptr;
+    }
+
+    RefPtr<HTMLTableRowElement> row = HTMLTableRowElement::create(document());
+    if (numRows == index || index == -1) {
+        appendChild(row, exceptionState);
     } else {
-        row = HTMLTableRowElement::create(document());
-        if (numRows == index || index == -1)
-            appendChild(row, exceptionState);
-        else {
-            Node* n;
-            if (index < 1)
-                n = firstChild();
-            else
-                n = children->item(index);
-            insertBefore(row, n, exceptionState);
-        }
+        Node* n = index ? children->item(index) : firstChild();

[arv (Not doing code reviews), 2014/05/06 17:53:42]

fistChild does not seem right here. It should be firstElementChild. You should
be able to test this with a whitespace text node as the firstChild.

"Otherwise, the method must create a tr element, insert it as a child of the
table section element, immediately before the indexth tr element in the rows
collection, and finally must return the newly created tr element."

[Inactive, 2014/05/06 17:57:54]

Oh, I thought this looked weird as well when I refactored this part of the code.
I will take investigate and fix if needed in a follow-up patch. Thanks for
noticing this.

[Inactive, 2014/05/06 18:55:45]

Yes, I confirmed we behave differently thatn Firefox and the spec here:
http://jsfiddle.net/JS4VU/1/

+        insertBefore(row, n, exceptionState);
     }
     return row.release();
 }
 
 void HTMLTableSectionElement::deleteRow(int index, ExceptionState& exceptionState)
 {
     RefPtr<HTMLCollection> children = rows();
     int numRows = children ? (int)children->length() : 0;
     if (index == -1)
         index = numRows - 1;
@@ skipping 12 matching lines @@
         ++rowCount;
     return rowCount;
 }
 
 PassRefPtr<HTMLCollection> HTMLTableSectionElement::rows()
 {
     return ensureCachedHTMLCollection(TSectionRows);
 }
 
 }