Bug 142253 - Implement -apple-trailing-word: -apple-partially-balanced
Summary: Implement -apple-trailing-word: -apple-partially-balanced
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-03-03 20:15 PST by Myles C. Maxfield
Modified: 2015-03-04 12:48 PST (History)
10 users (show)

See Also:


Attachments
Patch (130.17 KB, patch)
2015-03-03 20:33 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (45.76 KB, patch)
2015-03-03 20:47 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (45.87 KB, patch)
2015-03-03 20:50 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (26.35 KB, patch)
2015-03-04 12:41 PST, Myles C. Maxfield
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-03-03 20:15:41 PST
Implement -apple-trailing-word: -apple-partially-balanced
Comment 1 Myles C. Maxfield 2015-03-03 20:33:07 PST
Created attachment 247829 [details]
Patch
Comment 2 Myles C. Maxfield 2015-03-03 20:47:50 PST
Created attachment 247831 [details]
Patch
Comment 3 Myles C. Maxfield 2015-03-03 20:50:02 PST
Created attachment 247833 [details]
Patch
Comment 4 Myles C. Maxfield 2015-03-04 11:16:38 PST
<rdar://problem/19199732>
Comment 5 Dave Hyatt 2015-03-04 11:49:21 PST
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.
Comment 6 Myles C. Maxfield 2015-03-04 12:41:09 PST
Created attachment 247882 [details]
Patch
Comment 7 Dave Hyatt 2015-03-04 12:43:21 PST
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.
Comment 8 Myles C. Maxfield 2015-03-04 12:48:30 PST
Committed r181013: <http://trac.webkit.org/changeset/181013>