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

Side by Side 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 unified diff | Download patch | Annotate | Revision Log

« 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 »

OLD	NEW
1 /*	1 /*

2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)	2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)

3 * (C) 1999 Antti Koivisto (koivisto@kde.org)	3 * (C) 1999 Antti Koivisto (koivisto@kde.org)

4 * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.	4 * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.

5 *	5 *

6 * This library is free software; you can redistribute it and/or	6 * This library is free software; you can redistribute it and/or

7 * modify it under the terms of the GNU Library General Public	7 * modify it under the terms of the GNU Library General Public

8 * License as published by the Free Software Foundation; either	8 * License as published by the Free Software Foundation; either

9 * version 2 of the License, or (at your option) any later version.	9 * version 2 of the License, or (at your option) any later version.

10 *	10 *

(...skipping 230 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
241 static inline bool hasGeneratedAnonymousTableCells(const LayoutObject& parent)	241 static inline bool hasGeneratedAnonymousTableCells(const LayoutObject& parent)

242 {	242 {

243 // We're checking whether the table part has generated anonymous table	243 // We're checking whether the table part has generated anonymous table

244 // part wrappers to hold its contents, so inspecting its first child will su ffice.	244 // part wrappers to hold its contents, so inspecting its first child will su ffice.

245 LayoutObject* child = parent.slowFirstChild();	245 LayoutObject* child = parent.slowFirstChild();

246 if (!child \|\| !child->isAnonymous())	246 if (!child \|\| !child->isAnonymous())

247 return false;	247 return false;

248 if (child->isTableCell()) {	248 if (child->isTableCell()) {

249 LayoutObject* firstChild = child->slowFirstChild();	249 LayoutObject* firstChild = child->slowFirstChild();

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

251 return !firstChild \|\| !firstChild->node() \|\| !isHTMLTableCellElement(fir stChild->node());	251 return !firstChild \|\| !firstChild->node() \|\| (firstChild->style()->origi nalDisplay() != 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.
252 }	252 }

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

254 return hasGeneratedAnonymousTableCells(*child);	254 return hasGeneratedAnonymousTableCells(*child);

255 return false;	255 return false;

256 }	256 }

257	257

258 static inline bool canHaveWhitespaceChildren(const LayoutObject& parent)	258 static inline bool canHaveWhitespaceChildren(const LayoutObject& parent)

259 {	259 {

260 // <button> should allow whitespace even though LayoutFlexibleBox doesn't.	260 // <button> should allow whitespace even though LayoutFlexibleBox doesn't.

261 if (parent.isLayoutButton())	261 if (parent.isLayoutButton())

(...skipping 194 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
456 result.appendLiteral("; ");	456 result.appendLiteral("; ");

457 result.appendLiteral("value=");	457 result.appendLiteral("value=");

458 result.append(s);	458 result.append(s);

459 }	459 }

460	460

461 strncpy(buffer, result.toString().utf8().data(), length - 1);	461 strncpy(buffer, result.toString().utf8().data(), length - 1);

462 }	462 }

463 #endif	463 #endif

464	464

465 } // namespace blink	465 } // namespace blink

OLD	NEW