Use Traversal<HTMLElement> API in HTMLTableRowsCollection

Source/core/html/HTMLTableRowsCollection.cpp

 /*
  * Copyright (C) 2008, 2011, 2012 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 35 matching lines @@
 static bool isInBody(Element* row)
 {
     return row->parentNode() && toElement(row->parentNode())->hasLocalName(tbodyTag);
 }
 
 static bool isInFoot(Element* row)
 {
     return row->parentNode() && toElement(row->parentNode())->hasLocalName(tfootTag);
 }
 
-static inline HTMLTableRowElement* findTableRowElementInChildren(Element& current)
-{
-    return Traversal<HTMLTableRowElement>::firstChild(current);
-}
-
 HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement& table, HTMLTableRowElement* previous)
 {
-    Element* child = 0;
-
     // Start by looking for the next row in this section.
     // Continue only if there is none.
     if (previous && previous->parentNode() != table) {
         if (HTMLTableRowElement* row = Traversal<HTMLTableRowElement>::nextSibling(*previous))
             return row;
     }
 
     // If still looking at head sections, find the first row in the next head section.
+    HTMLElement* child = 0;
     if (!previous)
-        child = ElementTraversal::firstWithin(table);
+        child = Traversal<HTMLElement>::firstChild(table);
     else if (isInHead(previous))
-        child = ElementTraversal::nextSibling(*previous->parentNode());
-    for (; child; child = ElementTraversal::nextSibling(*child)) {
-        if (child->hasTagName(theadTag)) {
-            if (HTMLTableRowElement* row = findTableRowElementInChildren(*child))
+        child = Traversal<HTMLElement>::nextSibling(*previous->parentNode());
+    for (; child; child = Traversal<HTMLElement>::nextSibling(*child)) {
+        if (child->hasLocalName(theadTag)) {
+            if (HTMLTableRowElement* row = Traversal<HTMLTableRowElement>::firstChild(*child))
                 return row;
         }
     }
 
     // If still looking at top level and bodies, find the next row in top level or the first in the next body section.
     if (!previous || isInHead(previous))
-        child = ElementTraversal::firstWithin(table);
+        child = Traversal<HTMLElement>::firstChild(table);
     else if (previous->parentNode() == table)
-        child = ElementTraversal::nextSibling(*previous);
+        child = Traversal<HTMLElement>::nextSibling(*previous);
     else if (isInBody(previous))
-        child = ElementTraversal::nextSibling(*previous->parentNode());
-    for (; child; child = ElementTraversal::nextSibling(*child)) {
+        child = Traversal<HTMLElement>::nextSibling(*previous->parentNode());
+    for (; child; child = Traversal<HTMLElement>::nextSibling(*child)) {

[esprehn, 2014/03/27 13:35:49]

This isn't equivalent to the original code. If you had:


  <non-html>
     <span></span>
     <tr id="1">
     </tr>
  </non-html>
  <tr id="2"></td>

You return 1, it used to return 2. This might be okay, but you are changing
behavior.

[Inactive, 2014/03/27 13:57:58]

Arg, you are right, I caused a slight behavior change. This was unintentional, I
will address this in a follow-up today. Thanks for catching this.

[Inactive, 2014/03/27 14:39:54]

Actually, the new code returns 2 as well since I am calling
Traversal<HTMLElement>::firstChild(table) and not
Traversal<HTMLElement>::firstWithin(table). The new code will *not* traverse
descendants, only children.

Let me know if this is me misunderstanding your example.

         if (isHTMLTableRowElement(child))
             return toHTMLTableRowElement(child);
-        if (child->hasTagName(tbodyTag)) {
-            if (HTMLTableRowElement* row = findTableRowElementInChildren(*child))
+        if (child->hasLocalName(tbodyTag)) {
+            if (HTMLTableRowElement* row = Traversal<HTMLTableRowElement>::firstChild(*child))
                 return row;
         }
     }
 
     // Find the first row in the next foot section.
     if (!previous || !isInFoot(previous))
-        child = ElementTraversal::firstWithin(table);
+        child = Traversal<HTMLElement>::firstChild(table);
     else
-        child = ElementTraversal::nextSibling(*previous->parentNode());
-    for (; child; child = ElementTraversal::nextSibling(*child)) {
-        if (child->hasTagName(tfootTag)) {
-            if (HTMLTableRowElement* row = findTableRowElementInChildren(*child))
+        child = Traversal<HTMLElement>::nextSibling(*previous->parentNode());
+    for (; child; child = Traversal<HTMLElement>::nextSibling(*child)) {
+        if (child->hasLocalName(tfootTag)) {
+            if (HTMLTableRowElement* row = Traversal<HTMLTableRowElement>::firstChild(*child))
                 return row;
         }
     }
 
     return 0;
 }
 
 HTMLTableRowElement* HTMLTableRowsCollection::lastRow(HTMLTableElement& table)
 {
-    for (Node* child = table.lastChild(); child; child = child->previousSibling()) {
-        if (child->hasTagName(tfootTag)) {
+    for (HTMLElement* child = Traversal<HTMLElement>::lastChild(table); child; child = Traversal<HTMLElement>::previousSibling(*child)) {
+        if (child->hasLocalName(tfootTag)) {
             if (HTMLTableRowElement* lastRow = Traversal<HTMLTableRowElement>::lastChild(*child))
                 return lastRow;
         }
     }
 
-    for (Node* child = table.lastChild(); child; child = child->previousSibling()) {
+    for (HTMLElement* child = Traversal<HTMLElement>::lastChild(table); child; child = Traversal<HTMLElement>::previousSibling(*child)) {
         if (isHTMLTableRowElement(child))
             return toHTMLTableRowElement(child);
-        if (child->hasTagName(tbodyTag)) {
+        if (child->hasLocalName(tbodyTag)) {
             if (HTMLTableRowElement* lastRow = Traversal<HTMLTableRowElement>::lastChild(*child))
                 return lastRow;
         }
     }
 
-    for (Node* child = table.lastChild(); child; child = child->previousSibling()) {
-        if (child->hasTagName(theadTag)) {
+    for (HTMLElement* child = Traversal<HTMLElement>::lastChild(table); child; child = Traversal<HTMLElement>::previousSibling(*child)) {

[esprehn, 2014/03/27 13:35:49]

Why not use Traversal<HTMLTableSectionElement> ?

[Inactive, 2014/03/27 13:57:58]

We are only interested in thead elements so using
Traversal<HTMLTableSectionElement> would be a bit less efficient:
- we would do the following checks while iterating:
element.hasTagName(HTMLNames::tbodyTag) ||
element.hasTagName(HTMLNames::tfootTag) ||
element.hasTagName(HTMLNames::theadTag);

Then we would still have to do a child->hasLocalName(theadTag) inside the loop.

It is a bit unfortunate we don't have a specific type for thead elements (e.g.
HTMLTableHeadElement which would subclass HTMLTableSectionElement).

+        if (child->hasLocalName(theadTag)) {
             if (HTMLTableRowElement* lastRow = Traversal<HTMLTableRowElement>::lastChild(*child))
                 return lastRow;
         }
     }
 
     return 0;
 }
 
 // Must call get() on the table in case that argument is compiled before dereferencing the
 // table to get at the collection cache. Order of argument evaluation is undefined and can
 // differ between compilers.
 HTMLTableRowsCollection::HTMLTableRowsCollection(ContainerNode& table)
     : HTMLCollection(table, TableRows, OverridesItemAfter)
 {
     ASSERT(isHTMLTableElement(table));
 }
 
 PassRefPtr<HTMLTableRowsCollection> HTMLTableRowsCollection::create(ContainerNode& table, CollectionType)
 {
     return adoptRef(new HTMLTableRowsCollection(table));
 }
 
 Element* HTMLTableRowsCollection::virtualItemAfter(Element* previous) const
 {
     return rowAfter(toHTMLTableElement(ownerNode()), toHTMLTableRowElement(previous));
 }
 
 }