Commit 1ac4a650 authored by robhogan@gmail.com's avatar robhogan@gmail.com

Empty inline elements always get a line box.

CSS 2.1 (http://www.w3.org/TR/CSS2/visudet.html#leading) says: "Empty inline 
elements generate empty inline boxes, but these boxes still have margins, padding, 
borders and a line height, and thus influence [line-height calculation] just 
like elements with content."

We weren't creating lineboxes for empty inline elements when they occurred
between a single trailing space and a single leading space - this is because
our linebox creation was based on the too-narrow premise that we were already
ignoring spaces. However, if an empty inline element comes after a trailing 
space but before one or more leading spaces then it will be ignored as part 
of the collapsed space between the trailing space and the next non-collapsible 
space - so we should force the creation of a linebox in such cases.

This allows us to pass Hixie's empty inline test: 
http://www.hixie.ch/tests/evil/mixed/emptyinline.html

BUG=328939

Review URL: https://codereview.chromium.org/300853007

git-svn-id: svn://svn.chromium.org/blink/trunk@175205 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 1e265264
An empty inline should still get a linebox even when it precedes a space at which we start collapsing whitespace.
X X
PASS
X X
PASS
X X
PASS
X X
PASS
XX
PASS
X X
PASS
X X
PASS
<!DOCTYPE HTML>
<style>
#container {
font: 20px/1 Ahem;
border: solid orange;
}
</style>
<script src="../../resources/check-layout.js"></script>
<p>An empty inline should still get a linebox even when it precedes a space at which we start collapsing whitespace.</p>
<div id="container" style="width: 100px" data-expected-client-height=50>
X <span style="font-size: 50px;"></span> X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X<span style="font-size: 50px;"></span> X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X <span style="font-size: 50px;"></span>X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X <span style="font-size: 50px;"></span> X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X<span style="font-size: 50px;"></span>X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X<span style="font-size: 50px;"></span> X
</div>
<div id="container" style="width: 100px" data-expected-client-height=50>
X <span style="font-size: 50px;"></span> X
</div>
<script>
checkLayout('#container');
</script>
...@@ -48,7 +48,7 @@ InlineIterator BreakingContext::handleEndOfLine() ...@@ -48,7 +48,7 @@ InlineIterator BreakingContext::handleEndOfLine()
// Sanity check our midpoints. // Sanity check our midpoints.
m_lineMidpointState.checkMidpoints(m_lineBreak); m_lineMidpointState.checkMidpoints(m_lineBreak);
m_trailingObjects.updateMidpointsForTrailingBoxes(m_lineMidpointState, m_lineBreak, TrailingObjects::CollapseFirstSpace); m_trailingObjects.updateMidpointsForTrailingObjects(m_lineMidpointState, m_lineBreak, TrailingObjects::CollapseFirstSpace);
// We might have made lineBreak an iterator that points past the end // We might have made lineBreak an iterator that points past the end
// of the object. Do this adjustment to make it point to the start // of the object. Do this adjustment to make it point to the start
......
...@@ -362,7 +362,7 @@ inline void BreakingContext::handleOutOfFlowPositioned(Vector<RenderBox*>& posit ...@@ -362,7 +362,7 @@ inline void BreakingContext::handleOutOfFlowPositioned(Vector<RenderBox*>& posit
if (isInlineType || box->container()->isRenderInline()) { if (isInlineType || box->container()->isRenderInline()) {
if (m_ignoringSpaces) if (m_ignoringSpaces)
m_lineMidpointState.ensureLineBoxInsideIgnoredSpaces(box); m_lineMidpointState.ensureLineBoxInsideIgnoredSpaces(box);
m_trailingObjects.appendBoxIfNeeded(box); m_trailingObjects.appendObjectIfNeeded(box);
} else { } else {
positionedObjects.append(box); positionedObjects.append(box);
} }
...@@ -419,25 +419,27 @@ inline void BreakingContext::handleEmptyInline() ...@@ -419,25 +419,27 @@ inline void BreakingContext::handleEmptyInline()
RenderInline* flowBox = toRenderInline(m_current.object()); RenderInline* flowBox = toRenderInline(m_current.object());
// Now that some inline flows have line boxes, if we are already ignoring spaces, we need
// to make sure that we stop to include this object and then start ignoring spaces again.
// If this object is at the start of the line, we need to behave like list markers and
// start ignoring spaces.
bool requiresLineBox = alwaysRequiresLineBox(m_current.object()); bool requiresLineBox = alwaysRequiresLineBox(m_current.object());
if (requiresLineBox || requiresLineBoxForContent(flowBox, m_lineInfo)) { if (requiresLineBox || requiresLineBoxForContent(flowBox, m_lineInfo)) {
// An empty inline that only has line-height, vertical-align or font-metrics will only get a // An empty inline that only has line-height, vertical-align or font-metrics will
// line box to affect the height of the line if the rest of the line is not empty. // not force linebox creation (and thus affect the height of the line) if the rest of the line is empty.
if (requiresLineBox) if (requiresLineBox)
m_lineInfo.setEmpty(false, m_block, &m_width); m_lineInfo.setEmpty(false, m_block, &m_width);
if (m_ignoringSpaces) { if (m_ignoringSpaces) {
// If we are in a run of ignored spaces then ensure we get a linebox if lineboxes are eventually
// created for the line...
m_trailingObjects.clear(); m_trailingObjects.clear();
m_lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_current.object()); m_lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_current.object());
} else if (m_blockStyle->collapseWhiteSpace() && m_resolver.position().object() == m_current.object() } else if (m_blockStyle->collapseWhiteSpace() && m_resolver.position().object() == m_current.object()
&& shouldSkipWhitespaceAfterStartObject(m_block, m_current.object(), m_lineMidpointState)) { && shouldSkipWhitespaceAfterStartObject(m_block, m_current.object(), m_lineMidpointState)) {
// Like with list markers, we start ignoring spaces to make sure that any // If this object is at the start of the line, we need to behave like list markers and
// additional spaces we see will be discarded. // start ignoring spaces.
m_currentCharacterShouldCollapseIfPreWap = m_currentCharacterIsSpace = true; m_currentCharacterShouldCollapseIfPreWap = m_currentCharacterIsSpace = true;
m_ignoringSpaces = true; m_ignoringSpaces = true;
} else {
// If we are after a trailing space but aren't ignoring spaces yet then ensure we get a linebox
// if we encounter collapsible whitepace.
m_trailingObjects.appendObjectIfNeeded(m_current.object());
} }
} }
...@@ -769,7 +771,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool ...@@ -769,7 +771,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
// spaces. Create a midpoint to terminate the run // spaces. Create a midpoint to terminate the run
// before the second space. // before the second space.
m_lineMidpointState.startIgnoringSpaces(m_startOfIgnoredSpaces); m_lineMidpointState.startIgnoringSpaces(m_startOfIgnoredSpaces);
m_trailingObjects.updateMidpointsForTrailingBoxes(m_lineMidpointState, InlineIterator(), TrailingObjects::DoNotCollapseFirstSpace); m_trailingObjects.updateMidpointsForTrailingObjects(m_lineMidpointState, InlineIterator(), TrailingObjects::DoNotCollapseFirstSpace);
} }
} }
} else if (m_ignoringSpaces) { } else if (m_ignoringSpaces) {
......
...@@ -28,7 +28,7 @@ ...@@ -28,7 +28,7 @@
namespace WebCore { namespace WebCore {
void TrailingObjects::updateMidpointsForTrailingBoxes(LineMidpointState& lineMidpointState, const InlineIterator& lBreak, CollapseFirstSpaceOrNot collapseFirstSpace) void TrailingObjects::updateMidpointsForTrailingObjects(LineMidpointState& lineMidpointState, const InlineIterator& lBreak, CollapseFirstSpaceOrNot collapseFirstSpace)
{ {
if (!m_whitespace) if (!m_whitespace)
return; return;
...@@ -46,13 +46,13 @@ void TrailingObjects::updateMidpointsForTrailingBoxes(LineMidpointState& lineMid ...@@ -46,13 +46,13 @@ void TrailingObjects::updateMidpointsForTrailingBoxes(LineMidpointState& lineMid
// Now make sure every single trailingPositionedBox following the trailingSpaceMidpoint properly stops and starts // Now make sure every single trailingPositionedBox following the trailingSpaceMidpoint properly stops and starts
// ignoring spaces. // ignoring spaces.
size_t currentMidpoint = trailingSpaceMidpoint + 1; size_t currentMidpoint = trailingSpaceMidpoint + 1;
for (size_t i = 0; i < m_boxes.size(); ++i) { for (size_t i = 0; i < m_objects.size(); ++i) {
if (currentMidpoint >= lineMidpointState.numMidpoints()) { if (currentMidpoint >= lineMidpointState.numMidpoints()) {
// We don't have a midpoint for this box yet. // We don't have a midpoint for this box yet.
lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_boxes[i]); lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_objects[i]);
} else { } else {
ASSERT(lineMidpointState.midpoints()[currentMidpoint].object() == m_boxes[i]); ASSERT(lineMidpointState.midpoints()[currentMidpoint].object() == m_objects[i]);
ASSERT(lineMidpointState.midpoints()[currentMidpoint + 1].object() == m_boxes[i]); ASSERT(lineMidpointState.midpoints()[currentMidpoint + 1].object() == m_objects[i]);
} }
currentMidpoint += 2; currentMidpoint += 2;
} }
...@@ -64,8 +64,8 @@ void TrailingObjects::updateMidpointsForTrailingBoxes(LineMidpointState& lineMid ...@@ -64,8 +64,8 @@ void TrailingObjects::updateMidpointsForTrailingBoxes(LineMidpointState& lineMid
unsigned pos = length >= 2 ? length - 2 : UINT_MAX; unsigned pos = length >= 2 ? length - 2 : UINT_MAX;
InlineIterator endMid(0, m_whitespace, pos); InlineIterator endMid(0, m_whitespace, pos);
lineMidpointState.startIgnoringSpaces(endMid); lineMidpointState.startIgnoringSpaces(endMid);
for (size_t i = 0; i < m_boxes.size(); ++i) { for (size_t i = 0; i < m_objects.size(); ++i) {
lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_boxes[i]); lineMidpointState.ensureLineBoxInsideIgnoredSpaces(m_objects[i]);
} }
} }
} }
......
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
namespace WebCore { namespace WebCore {
class InlineIterator; class InlineIterator;
class RenderBox; class RenderObject;
class RenderText; class RenderText;
struct BidiRun; struct BidiRun;
...@@ -39,6 +39,19 @@ template <class Iterator> class MidpointState; ...@@ -39,6 +39,19 @@ template <class Iterator> class MidpointState;
typedef BidiResolver<InlineIterator, BidiRun> InlineBidiResolver; typedef BidiResolver<InlineIterator, BidiRun> InlineBidiResolver;
typedef MidpointState<InlineIterator> LineMidpointState; typedef MidpointState<InlineIterator> LineMidpointState;
// This class allows us to ensure lineboxes are created in the right place on the line when
// an out-of-flow positioned object or an empty inline is encountered between a trailing space
// and subsequent spaces and we want to ignore (i.e. collapse) surplus whitespace. So for example:
// <div>X <span></span> Y</div>
// or
// <div>X <div style="position: absolute"></div> Y</div>
// In both of the above snippets the inline and the positioned object occur after a trailing space
// and before a space that will cause our line breaking algorithm to start ignoring spaces. When it
// does that we want to ensure that the inline/positioned object gets a linebox and that it is part
// of the collapsed whitespace. So to achieve this we use appendObjectIfNeeded() to keep track of
// objects encountered after a trailing whitespace and updateMidpointsForTrailingObjects() to put
// them in the right place when we start ignoring surplus whitespace.
class TrailingObjects { class TrailingObjects {
public: public:
TrailingObjects() TrailingObjects()
...@@ -58,22 +71,22 @@ public: ...@@ -58,22 +71,22 @@ public:
// Using resize(0) rather than clear() here saves 2% on // Using resize(0) rather than clear() here saves 2% on
// PerformanceTests/Layout/line-layout.html because we avoid freeing and // PerformanceTests/Layout/line-layout.html because we avoid freeing and
// re-allocating the underlying buffer repeatedly. // re-allocating the underlying buffer repeatedly.
m_boxes.resize(0); m_objects.resize(0);
} }
void appendBoxIfNeeded(RenderBox* box) void appendObjectIfNeeded(RenderObject* object)
{ {
if (m_whitespace) if (m_whitespace)
m_boxes.append(box); m_objects.append(object);
} }
enum CollapseFirstSpaceOrNot { DoNotCollapseFirstSpace, CollapseFirstSpace }; enum CollapseFirstSpaceOrNot { DoNotCollapseFirstSpace, CollapseFirstSpace };
void updateMidpointsForTrailingBoxes(LineMidpointState&, const InlineIterator& lBreak, CollapseFirstSpaceOrNot); void updateMidpointsForTrailingObjects(LineMidpointState&, const InlineIterator& lBreak, CollapseFirstSpaceOrNot);
private: private:
RenderText* m_whitespace; RenderText* m_whitespace;
Vector<RenderBox*, 4> m_boxes; Vector<RenderObject*, 4> m_objects;
}; };
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment