Allow whitespace inside anonymous table cells

Source/core/dom/Text.cpp

Index: Source/core/dom/Text.cpp
diff --git a/Source/core/dom/Text.cpp b/Source/core/dom/Text.cpp
index d48c27e8cd5c3a82c78f874b84b63c0c2e0b9177..1cb04e97c10f5e9ee90ab37259d3ee326dc8d647 100644
--- a/Source/core/dom/Text.cpp
+++ b/Source/core/dom/Text.cpp
@@ -272,14 +272,34 @@ 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()