Allow whitespace inside anonymous table cells

Source/core/dom/Text.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 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
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 254 matching lines @@
 Node::NodeType Text::nodeType() const
 {
     return TEXT_NODE;
 }
 
 PassRefPtrWillBeRawPtr<Node> Text::cloneNode(bool /*deep*/)
 {
     return cloneWithData(data());
 }
 
+static inline bool isTableCellAncestor(const LayoutObject& renderer)

[esprehn, 2015/05/04 22:12:16]

not inline. also don't use the word renderer. Maybe layouter?

+{
+    return renderer.isTable() || renderer.isTableRow() || renderer.isTableSection() || renderer.isLayoutTableCol();

[esprehn, 2015/05/04 22:12:16]

LayoutTableCol layouters only ever have more LayoutTableCol children and nothing
else. They can't contain whitespace.

[rhogan, 2015/05/05 21:18:49]

Right, this makes such cases slightly more expensive though they ultimately
return the correct result because no cell is found.

[esprehn, 2015/05/28 05:43:55]

The code doesn't make sense though, you can't find whitespace or rows there why
check at all?

+}
+
+static inline bool isAncestorOfAnonymousTableCell(const LayoutObject& parent)

[esprehn, 2015/05/04 22:12:16]

not inline.

+{
+    LayoutObject* firstChild = parent.slowFirstChild();
+    if (!firstChild || !firstChild->isAnonymous())
+        return false;
+    if (firstChild->isTableCell())
+        return true;
+    if (isTableCellAncestor(*firstChild))
+        return isAncestorOfAnonymousTableCell(*firstChild);

[esprehn, 2015/05/04 22:12:16]

This check doesn't make sense, why are you drilling down the firstChild pointers
of the tree? This shouldn't be recursive.

[rhogan, 2015/05/05 21:18:49]

If the table part is the ancestor of an anonymous table cell (i.e. the
whitespace would end up in a table cell) then the cell will be a descendant of
the table part through the first child of each ancestor.

+    return false;
+}
+
 static inline bool canHaveWhitespaceChildren(const LayoutObject& parent)
 {
     // <button> should allow whitespace even though LayoutFlexibleBox doesn't.
     if (parent.isLayoutButton())
         return true;
 
-    if (parent.isTable() || parent.isTableRow() || parent.isTableSection()
-        || parent.isLayoutTableCol() || parent.isFrameSet()
+    // Allow whitespace when the text will be inside an anonymous table cell.
+    if (isAncestorOfAnonymousTableCell(parent))

[leviw_travelin_and_unemployed, 2015/04/21 18:42:37]

This is more expensive in the common case, and this path can be hot. Do we know
of any actual web content that's effected by this?

[rhogan, 2015/04/21 19:00:48]

It's not that much more expensive to my reading. In the common case it will hit
isTableCellAncestor and return false, so three extra checks.

Greg from MS raised it - he usually does that on the basis of an
interoperability bug reported to them. I'll ping him on the crbug.

+        return true;
+
+    if (isTableCellAncestor(parent) || parent.isFrameSet()
         || parent.isFlexibleBox() || parent.isLayoutGrid()
         || parent.isSVGRoot()
         || parent.isSVGContainer()
         || parent.isSVGImage()
         || parent.isSVGShape())
         return false;
     return true;
 }
 
 bool Text::textRendererIsNeeded(const ComputedStyle& style, const LayoutObject& parent)
@@ skipping 172 matching lines @@
             result.appendLiteral("; ");
         result.appendLiteral("value=");
         result.append(s);
     }
 
     strncpy(buffer, result.toString().utf8().data(), length - 1);
 }
 #endif
 
 } // namespace blink