Source/core/dom/Text.cpp - Issue 1280123004: Don't allow whitespace between elements with display:table-cell

Unified Diff: Source/core/dom/Text.cpp

Issue 1280123004: Don't allow whitespace between elements with display:table-cell (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: Updated Created 5 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: Source/core/dom/Text.cpp

diff --git a/Source/core/dom/Text.cpp b/Source/core/dom/Text.cpp

index cbe43473484023544e5c5095819cd984c78a5cf2..77ced4244ebf32a6a75f189b6e5a36db2a9d67fd 100644

--- a/Source/core/dom/Text.cpp

+++ b/Source/core/dom/Text.cpp

@@ -248,7 +248,7 @@ static inline bool hasGeneratedAnonymousTableCells(const LayoutObject& parent)

if (child->isTableCell()) {

LayoutObject* firstChild = child->slowFirstChild();

// Ignore the anonymous table cell if it is wrapping a table cell element (e.g. because of <td style="display:block;">).

- return !firstChild || !firstChild->node() || !isHTMLTableCellElement(firstChild->node());

+ return !firstChild || !firstChild->node() || (firstChild->style()->originalDisplay() != TABLE_CELL && !isHTMLTableCellElement(firstChild->node()));

mstensho (USE GERRIT) 2015/08/25 11:24:05 I don't think this is quite right, because: <!DOC

I don't think this is quite right, because: <!DOCTYPE html> <style> input, button { display: table-cell; width: 50px; height: 100px; background: blue; border: none; padding: 0; vertical-align: bottom; } </style> <p>There should be a blue square below.</p> <div style="display:table;"> <div></div> <input> <button></button> </div> The first child here is a regular DIV, which causes us to return true. It fails to detect the table-cell displayed siblings. This is browser interoperability hell. They all prevent whitespace between the input and the button, but for different reasons. MSIE and Presto actually lay out the button and input elements as table-cells, it seems, while Gecko just somehow manages to prevent whitespace between them, without treating them as table cells. I think MSIE and Presto are right (why should they not become table cells, when they have display:table-cell?), but that's hard to achieve in Blink, since there's no display-outer / display-inner separation in the implementation (Presto has had this since day one, more or less). A table cell cannot be both a LayoutTableCell and LayoutTextControl at the same time, for instance. So, yeah, our best bet would be to do exactly like Gecko, which apparently is to not lay them out as table cells, but somehow prevent space between them. This change is introducing a regression: <!DOCTYPE html> <p>There should be two words (with space between them) below.</p> <div style="display:table;"> <input style="display:table-cell; background:transparent; border:none;"> <span>two</span> <span>words</span> </div> Finally, I have a general remark about this code (which should be dealt with in a separate CL): Why are we returning true if the table cell is childless? Why would we create an anonymous table cell around ... NOTHING? Also, the name hasGeneratedAnonymousTableCells() is a bit inaccurate, since it only returns true in certain cases for anonymous table cells.

}

if (child->isTableSection() || child->isTableRow())

return hasGeneratedAnonymousTableCells(*child);

« no previous file with comments | « LayoutTests/fast/table/whitespace-between-elements-with-table-cell-display-expected.html ('k') | no next file » | no next file with comments »