Bug 153980 - Soft hyphen is not shown when it is placed at the end of an inline element
Summary: Soft hyphen is not shown when it is placed at the end of an inline element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-08 02:33 PST by Roman Komarov
Modified: 2016-02-18 15:48 PST (History)
11 users (show)

See Also:


Attachments
Patch (9.20 KB, patch)
2016-02-18 09:58 PST, zalan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-yosemite (969.78 KB, application/zip)
2016-02-18 10:44 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1021.47 KB, application/zip)
2016-02-18 10:48 PST, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-yosemite (1.00 MB, application/zip)
2016-02-18 10:57 PST, Build Bot
no flags Details
Patch (9.21 KB, patch)
2016-02-18 14:14 PST, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Komarov 2016-02-08 02:33:52 PST
Test case: http://codepen.io/kizu/pen/bEQNVL

When there is a soft hyphen at the end an inline element and there is a line break at it, the hyphen is not shown. This prevents authors from styling the soft hyphens only, as it is impossible to wrap them into their own element to apply styles, like `mmmmm<span class="soft-hyphen">&shy;</span>mmmmmmmm`.

In other browsers (Fx, Edge) everything works as intended.
Comment 1 zalan 2016-02-17 14:01:50 PST
We missed the case when the character at the breaking position does not fit the line and soft-hyphen is followed by this overflowing character. (foo&shy;bar where b overflows the line). In such cases we don't yet have an item in the breaking history.      
This should fix it ->
diff --git a/Source/WebCore/rendering/line/BreakingContext.h b/Source/WebCore/rendering/line/BreakingContext.h
index b44c5a2..46b1c68 100644
--- a/Source/WebCore/rendering/line/BreakingContext.h
+++ b/Source/WebCore/rendering/line/BreakingContext.h
@@ -903,8 +903,28 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
                         m_lineInfo.setPreviousLineBrokeCleanly(true);
                         wordMeasurement.endOffset = m_lineBreakHistory.offset();
                     }
-                    if (m_lineBreakHistory.offset() && downcast<RenderText>(m_lineBreakHistory.renderer()) && downcast<RenderText>(*m_lineBreakHistory.renderer()).textLength() && downcast<RenderText>(*m_lineBreakHistory.renderer()).characterAt(m_lineBreakHistory.offset() - 1) == softHyphen && style.hyphens() != HyphensNone)
-                        hyphenated = true;
+                    // Check if the last breaking position is a soft-hyphen.
+                    if (!hyphenated) {
+                        const RenderText* textRenderer = nullptr;
+                        Optional<int> breakingPositon;
+                        if (m_lineBreakHistory.historyLength() && is<RenderText>(m_lineBreakHistory.renderer())) {
+                            textRenderer = downcast<RenderText>(m_lineBreakHistory.renderer());
+                            breakingPositon = m_lineBreakHistory.offset();
+                        } else if (nextBreakablePosition > -1 && is<RenderText>(m_current.renderer())) {
+                            textRenderer = downcast<RenderText>(m_current.renderer());
+                            breakingPositon = nextBreakablePosition;
+                        }
+                        if (textRenderer && breakingPositon) {
+                            if (breakingPositon.value() == 0) {
+                                // We need to check the previous renderer for the soft-hyphen character instead.
+                                textRenderer = is<RenderText>(m_lastObject) ? downcast<RenderText>(m_lastObject) : nullptr;
+                                if (textRenderer)
+                                    breakingPositon = textRenderer->textLength();
+                            }
+                            UChar characterBeforeBreakingPosition = textRenderer->characterAt(breakingPositon.value() - 1);
+                            hyphenated = characterBeforeBreakingPosition == softHyphen && style.hyphens() != HyphensNone;
+                        }
+                    }
                     if (m_lineBreakHistory.offset() && m_lineBreakHistory.offset() != (unsigned)wordMeasurement.endOffset && !wordMeasurement.width) {
                         if (charWidth) {
                             wordMeasurement.endOffset = m_lineBreakHistory.offset();
Patch is coming up soon. (need to see first if checking prev is sufficient enough)
Comment 2 zalan 2016-02-18 09:58:44 PST
Created attachment 271665 [details]
Patch
Comment 3 Build Bot 2016-02-18 10:44:27 PST
Comment on attachment 271665 [details]
Patch

Attachment 271665 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/850622

New failing tests:
fast/text/midword-break-after-breakable-char.html
Comment 4 Build Bot 2016-02-18 10:44:32 PST
Created attachment 271668 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Dave Hyatt 2016-02-18 10:46:37 PST
Comment on attachment 271665 [details]
Patch

r=me
Comment 6 Build Bot 2016-02-18 10:48:42 PST
Comment on attachment 271665 [details]
Patch

Attachment 271665 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/850626

New failing tests:
fast/text/midword-break-after-breakable-char.html
Comment 7 Build Bot 2016-02-18 10:48:47 PST
Created attachment 271671 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-02-18 10:57:12 PST
Comment on attachment 271665 [details]
Patch

Attachment 271665 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/850625

New failing tests:
fast/text/midword-break-after-breakable-char.html
Comment 9 Build Bot 2016-02-18 10:57:17 PST
Created attachment 271672 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 zalan 2016-02-18 14:14:39 PST
Created attachment 271693 [details]
Patch
Comment 11 WebKit Commit Bot 2016-02-18 15:48:16 PST
Comment on attachment 271693 [details]
Patch

Clearing flags on attachment: 271693

Committed r196782: <http://trac.webkit.org/changeset/196782>
Comment 12 WebKit Commit Bot 2016-02-18 15:48:21 PST
All reviewed patches have been landed.  Closing bug.