Bug 142253 – Implement -apple-trailing-word: -apple-partially-balanced

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Implement -apple-trailing-word: -apple-partially-balanced

[Myles C. Maxfield]

Created attachment 247829 [details]
Patch

[Myles C. Maxfield]

Created attachment 247831 [details]
Patch

[Myles C. Maxfield]

Created attachment 247833 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/19199732>

[Dave Hyatt]

Comment on attachment 247833 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247833&action=review

Just one change.

> Source/WebCore/rendering/line/BreakingContextInlineHeaders.h:1185
> +#if ENABLE(CSS_TRAILING_WORD)
> +    if (m_blockStyle.trailingWord() == TrailingWord::PartiallyBalanced) {
> +        InlineIterator lineBreak = m_lineBreakHistory.current();
> +        if (lineBreak.renderer() && m_lineInfo.isFirstLine() && !bidiNextSkippingEmptyInlines(*lineBreak.root(), lineBreak.renderer()) && is<RenderText>(lineBreak.renderer())) {
> +            RenderText& renderText = downcast<RenderText>(*lineBreak.renderer());
> +            // Don't even bother measuring if our remaining line has many characters
> +            if (renderText.textLength() == lineBreak.offset() || renderText.textLength() - lineBreak.offset() > 20)
> +                return lineBreak;
> +            bool isLooseCJKMode = m_renderTextInfo.m_text != &renderText && m_renderTextInfo.m_lineBreakIterator.isLooseCJKMode();
> +            bool breakNBSP = m_autoWrap && m_currentStyle->nbspMode() == SPACE;
> +            int nextBreakablePosition = lineBreak.nextBreakablePosition();
> +            isBreakable(m_renderTextInfo.m_lineBreakIterator, lineBreak.offset() + 1, nextBreakablePosition, breakNBSP, isLooseCJKMode);
> +            if (nextBreakablePosition < 0 || static_cast<unsigned>(nextBreakablePosition) != renderText.textLength())
> +                return lineBreak;
> +            const RenderStyle& style = lineStyle(renderText, m_lineInfo);
> +            const FontCascade& font = style.fontCascade();
> +            HashSet<const Font*> dummyFonts;
> +            InlineIterator best = lineBreak;
> +            for (size_t i = 1; i < m_lineBreakHistory.historyLength(); ++i) {
> +                const InlineIterator& candidate = m_lineBreakHistory.get(i);
> +                if (candidate.renderer() != lineBreak.renderer())
> +                    return best;
> +                float width = textWidth(renderText, candidate.offset(), renderText.textLength() - candidate.offset(), font, 0, font.isFixedPitch(), m_collapseWhiteSpace, dummyFonts);
> +                if (width > m_width.availableWidth())
> +                    return best;
> +                if (width / m_width.availableWidth() > 0.1) // Subsequent line is long enough
> +                    return candidate;
> +                best = candidate;
> +            }
> +            return best;
> +        }
> +    }
> +#endif

This is long enough that it should just be put in its own helper function.

[Myles C. Maxfield]

Created attachment 247882 [details]
Patch

[Dave Hyatt]

Comment on attachment 247882 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247882&action=review

r=me

> Source/WebCore/rendering/line/BreakingContextInlineHeaders.h:1172
> +    if (renderText.textLength() == lineBreak.offset() || renderText.textLength() - lineBreak.offset() > 20)
> +        return lineBreak;

You should use a defined constant for the value 20 rather than just putting the raw number in this function.

[Myles C. Maxfield]

Committed r181013: <http://trac.webkit.org/changeset/181013>