Skip to content

Commit

Permalink
Revert of Add tab support to RenderBlockLineLayout::stripTrailingSpac…
Browse files Browse the repository at this point in the history
…e (patchset #1 id:1 of https://codereview.chromium.org/836863002/)

Reason for revert:
trailing_whitespace_wrapping.html is crashing on Mac10.6 (dbg)

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Ftext%2Ftrailing_whitespace_wrapping.html%2Cvirtual%2Fantialiasedtext%2Ffast%2Ftext%2Ftrailing_whitespace_wrapping.html%2Cvirtual%2Fslimmingpaint%2Ffast%2Ftext%2Ftrailing_whitespace_wrapping.html

Example:
22:23:47.008 3912 worker/4 fast/text/trailing_whitespace_wrapping.html crashed, (stderr lines):
22:23:47.008 3912   Assertion failed: (bool (status_and & kCTRunStatusRightToLeft) == backward), function _hb_coretext_shape, file ../../third_party/harfbuzz-ng/src/hb-coretext.cc, line 989.
22:23:47.009 3881 [30380/38782] fast/text/trailing_whitespace_wrapping.html failed unexpectedly (renderer crashed [pid=6953])

Original issue's description:
> Add tab support to RenderBlockLineLayout::stripTrailingSpace
> 
> Change RenderBlockLineLayout::stripTrailingSpace to measure the width of
> the first trailing white-space character instead of always assuming it's
> a space character. This fixes a bug where we wrap text unnecessarily for
> elements with a trailing tab character as we correctly measure the right
> character (tab) when computing the preferred with but subtract the width
> of a space character in stripTrailingSpace.
> 
> R=pdr@chromium.org
> BUG=444544
> TEST=fast/text/trailing_whitespace_wrapping.html
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187878

TBR=pdr@chromium.org,eae@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=444544

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

git-svn-id: svn://svn.chromium.org/blink/trunk@187897 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
mfalken committed Jan 6, 2015
1 parent 5d5863b commit 21f9881
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 83 deletions.
32 changes: 0 additions & 32 deletions LayoutTests/fast/text/trailing_whitespace_wrapping-expected.html

This file was deleted.

32 changes: 0 additions & 32 deletions LayoutTests/fast/text/trailing_whitespace_wrapping.html

This file was deleted.

27 changes: 8 additions & 19 deletions Source/core/rendering/RenderBlockLineLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,28 +1164,17 @@ static LayoutUnit getBorderPaddingMargin(RenderBoxModelObject* child, bool endOf
child->borderStart();
}

static inline void stripTrailingSpace(float& inlineMax, float& inlineMin,
RenderObject* trailingSpaceChild)
static inline void stripTrailingSpace(float& inlineMax, float& inlineMin, RenderObject* trailingSpaceChild)
{
if (trailingSpaceChild && trailingSpaceChild->isText()) {
// Collapse away the trailing space at the end of a block by finding
// the first white-space character and subtracting its width. Subsequent
// white-space characters have been collapsed into the first one (which
// can be either a space or a tab character).
// Collapse away the trailing space at the end of a block.
RenderText* text = toRenderText(trailingSpaceChild);
UChar trailingWhitespaceChar = ' ';
for (unsigned i = text->textLength(); i > 0; i--) {
UChar c = text->characterAt(i - 1);
if (!Character::treatAsSpace(c))
break;
trailingWhitespaceChar = c;
}

// FIXME: This ignores first-line.
const Font& font = text->style()->font();
TextRun run = constructTextRun(text, font, &trailingWhitespaceChar, 1,
text->style(), text->style()->direction());
run.setUseComplexCodePath(!text->canUseSimpleFontCodePath());
bool useComplexCodePath = !text->canUseSimpleFontCodePath();
const UChar space = ' ';
const Font& font = text->style()->font(); // FIXME: This ignores first-line.
TextRun run = constructTextRun(text, font, &space, 1, text->style(), LTR);
if (useComplexCodePath)
run.setUseComplexCodePath(true);
float spaceWidth = font.width(run);
inlineMax -= spaceWidth + font.fontDescription().wordSpacing();
if (inlineMin > inlineMax)
Expand Down

0 comments on commit 21f9881

Please sign in to comment.