Changeset 243437 in webkit


Ignore:
Timestamp:
Mar 25, 2019 8:07:35 AM (5 years ago)
Author:
jfernandez@igalia.com
Message:

A single leading space is not considered as a word break even when word-break: break-all is set
​https://bugs.webkit.org/show_bug.cgi?id=195361

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Imported additonal WPT from the CSS Text Suite that verify the
change doesn't regress in any case, specially for the word-break:
break-word feature.

  • web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007-expected.html: Added.
  • web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007.html: Added.
  • web-platform-tests/css/css-text/overflow-wrap/w3c-import.log:
  • web-platform-tests/css/css-text/white-space/pre-wrap-008-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/pre-wrap-008.html: Added.
  • web-platform-tests/css/css-text/white-space/w3c-import.log:
  • web-platform-tests/css/css-text/word-break/w3c-import.log:
  • web-platform-tests/css/css-text/word-break/word-break-break-all-015-expected.html: Added.
  • web-platform-tests/css/css-text/word-break/word-break-break-all-015.html: Added.

Source/WebCore:

We must consider leading white-spaces as potential soft-breaking
opportunities that may avoid breaking in the middle of the word.

However, 'break-word: break-all' [1] implies that we should ignore
previous opportunities and break at any character (among the ones
valid for 'break-all') that prevents the line to overflow. Note,
that these breakable characters are different from the ones
provided by 'line-break: anywhere' [2].

This change is covered by the already existent tests of the CSS
Text 3 suite of the Web Platform Tests.

The word-break-break-all-010.html was precisely designed to cover
the basic issue fixed with this change, verifying that the word is
indeed broken even if a single leading space constitutes a
previous soft-breaking opportunity.

There are other Web Platform Tests. which already pass before this
change, to verify that such leading white-space must be used
instead of breaking the word in any other case, including
overflow-wrap: break-word and even the deprecated word-break:
break-word.

  • white-space/pre-wrap-008.html
  • white-space/pre-wrap-015.html
  • white-space/pre-wrap-016.html
  • overflow-wrap/overflow-wrap-break-word-004.html
  • overflow-wrap/overflow-wrap-break-word-005.html
  • overflow-wrap/overflow-wrap-break-word-007.html
  • word-break/word-break-break-all-011.html
  • word-break/word-break-break-all-014.html

The reason why the word-break-break-all-010.html passes in Mac
platform is that for that case the SimpleLineLayout codepath is
executed instead, which doesn't have this bug, present in the old
line-breaking logic implemented in the BreakingContext class.

In order to verify the validity of this change, I've added several
tests under fast/text with the SimpleLineLayout disabled.

Tests: fast/text/overflow-wrap-break-word-004.html

fast/text/overflow-wrap-break-word-005.html
fast/text/overflow-wrap-break-word-007.html
fast/text/whitespace/pre-wrap-008.html
fast/text/whitespace/pre-wrap-015.html
fast/text/whitespace/pre-wrap-016.html
fast/text/word-break-break-all-010.html
fast/text/word-break-break-all-011.html
fast/text/word-break-break-all-015.html
imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007.html
imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-008.html
imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-015.html

  • rendering/line/BreakingContext.h:

(WebCore::BreakingContext::handleText):

LayoutTests:

Removed some entries from the GTK expectation file.
Added tests to verify the codepath with SimpleLineLayout disabled.

  • fast/text/overflow-wrap-break-word-004-expected.html: Added.
  • fast/text/overflow-wrap-break-word-004.html: Added.
  • fast/text/overflow-wrap-break-word-005-expected.html: Added.
  • fast/text/overflow-wrap-break-word-005.html: Added.
  • fast/text/overflow-wrap-break-word-007-expected.html: Added.
  • fast/text/overflow-wrap-break-word-007.html: Added.
  • fast/text/whitespace/pre-wrap-008-expected.html: Added.
  • fast/text/whitespace/pre-wrap-008.html: Added.
  • fast/text/whitespace/pre-wrap-015-expected.html: Added.
  • fast/text/whitespace/pre-wrap-015.html: Added.
  • fast/text/whitespace/pre-wrap-016-expected.html: Added.
  • fast/text/whitespace/pre-wrap-016.html: Added.
  • fast/text/word-break-break-all-010-expected.html: Added.
  • fast/text/word-break-break-all-010.html: Added.
  • fast/text/word-break-break-all-011-expected.html: Added.
  • fast/text/word-break-break-all-011.html: Added.
  • fast/text/word-break-break-all-015-expected.html: Added.
  • fast/text/word-break-break-all-015.html: Added.
  • platform/gtk/TestExpectations:
    • word-break-break-all-010.html passes now thanks to this change.
Location:
trunk
Files:
24 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243436 r243437  
     12019-03-25  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        A single leading space is not considered as a word break even when word-break: break-all is set
     4        https://bugs.webkit.org/show_bug.cgi?id=195361
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Removed some entries from the GTK expectation file.
     9        Added tests to verify the codepath with SimpleLineLayout disabled.
     10
     11        * fast/text/overflow-wrap-break-word-004-expected.html: Added.
     12        * fast/text/overflow-wrap-break-word-004.html: Added.
     13        * fast/text/overflow-wrap-break-word-005-expected.html: Added.
     14        * fast/text/overflow-wrap-break-word-005.html: Added.
     15        * fast/text/overflow-wrap-break-word-007-expected.html: Added.
     16        * fast/text/overflow-wrap-break-word-007.html: Added.
     17        * fast/text/whitespace/pre-wrap-008-expected.html: Added.
     18        * fast/text/whitespace/pre-wrap-008.html: Added.
     19        * fast/text/whitespace/pre-wrap-015-expected.html: Added.
     20        * fast/text/whitespace/pre-wrap-015.html: Added.
     21        * fast/text/whitespace/pre-wrap-016-expected.html: Added.
     22        * fast/text/whitespace/pre-wrap-016.html: Added.
     23        * fast/text/word-break-break-all-010-expected.html: Added.
     24        * fast/text/word-break-break-all-010.html: Added.
     25        * fast/text/word-break-break-all-011-expected.html: Added.
     26        * fast/text/word-break-break-all-011.html: Added.
     27        * fast/text/word-break-break-all-015-expected.html: Added.
     28        * fast/text/word-break-break-all-015.html: Added.
     29        * platform/gtk/TestExpectations:
     30          - word-break-break-all-010.html passes now thanks to this change.
     31
    1322019-03-25  Diego Pino Garcia  <dpino@igalia.com>
    233

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r243435 r243437  
     12019-03-25  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        A single leading space is not considered as a word break even when word-break: break-all is set
     4        https://bugs.webkit.org/show_bug.cgi?id=195361
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Imported additonal WPT from the CSS Text Suite that verify the
     9        change doesn't regress in any case, specially for the word-break:
     10        break-word feature.
     11
     12        * web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007-expected.html: Added.
     13        * web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007.html: Added.
     14        * web-platform-tests/css/css-text/overflow-wrap/w3c-import.log:
     15        * web-platform-tests/css/css-text/white-space/pre-wrap-008-expected.html: Added.
     16        * web-platform-tests/css/css-text/white-space/pre-wrap-008.html: Added.
     17        * web-platform-tests/css/css-text/white-space/w3c-import.log:
     18        * web-platform-tests/css/css-text/word-break/w3c-import.log:
     19        * web-platform-tests/css/css-text/word-break/word-break-break-all-015-expected.html: Added.
     20        * web-platform-tests/css/css-text/word-break/word-break-break-all-015.html: Added.
     21
    1222019-03-25  Rob Buis  <rbuis@igalia.com>
    223

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/w3c-import.log

    r242550 r243437  
    4848/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-006-expected.html
    4949/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-006.html
     50/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007-expected.html
     51/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007.html
    5052/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-fit-content-001-expected.html
    5153/LayoutTests/imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-fit-content-001.html

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/w3c-import.log

    r242550 r243437  
    173173/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-007-expected.html
    174174/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-007.html
     175/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-008-expected.html
     176/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-008.html
    175177/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-011-expected.html
    176178/LayoutTests/imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-011.html

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/w3c-import.log

    r242550 r243437  
    4343/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-014-expected.html
    4444/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-014.html
     45/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-015-expected.html
     46/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-015.html
    4547/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-020-expected.html
    4648/LayoutTests/imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-020.html

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r243436 r243437  
    35003500webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html [ ImageOnlyFailure ]
    35013501
    3502 webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-010.html [ ImageOnlyFailure ]
    35033502webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-014.html [ ImageOnlyFailure ]
    35043503

  • trunk/Source/WebCore/ChangeLog

    r243435 r243437  
     12019-03-25  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        A single leading space is not considered as a word break even when word-break: break-all is set
     4        https://bugs.webkit.org/show_bug.cgi?id=195361
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        We must consider leading white-spaces as potential soft-breaking
     9        opportunities that may avoid breaking in the middle of the word.
     10
     11        However, 'break-word: break-all' [1] implies that we should ignore
     12        previous opportunities and break at any character (among the ones
     13        valid for 'break-all') that prevents the line to overflow. Note,
     14        that these breakable characters are different from the ones
     15        provided by 'line-break: anywhere' [2].
     16
     17        This change is covered by the already existent tests of the CSS
     18        Text 3 suite of the Web Platform Tests.
     19
     20        The word-break-break-all-010.html was precisely designed to cover
     21        the basic issue fixed with this change, verifying that the word is
     22        indeed broken even if a single leading space constitutes a
     23        previous soft-breaking opportunity.
     24
     25        There are other Web Platform Tests. which already pass before this
     26        change, to verify that such leading white-space must be used
     27        instead of breaking the word in any other case, including
     28        overflow-wrap: break-word and even the deprecated word-break:
     29        break-word.
     30
     31           - white-space/pre-wrap-008.html
     32           - white-space/pre-wrap-015.html
     33           - white-space/pre-wrap-016.html
     34           - overflow-wrap/overflow-wrap-break-word-004.html
     35           - overflow-wrap/overflow-wrap-break-word-005.html
     36           - overflow-wrap/overflow-wrap-break-word-007.html
     37           - word-break/word-break-break-all-011.html
     38           - word-break/word-break-break-all-014.html
     39
     40        The reason why the word-break-break-all-010.html passes in Mac
     41        platform is that for that case the SimpleLineLayout codepath is
     42        executed instead, which doesn't have this bug, present in the old
     43        line-breaking logic implemented in the BreakingContext class.
     44
     45        In order to verify the validity of this change, I've added several
     46        tests under fast/text with the SimpleLineLayout disabled.
     47
     48        Tests: fast/text/overflow-wrap-break-word-004.html
     49               fast/text/overflow-wrap-break-word-005.html
     50               fast/text/overflow-wrap-break-word-007.html
     51               fast/text/whitespace/pre-wrap-008.html
     52               fast/text/whitespace/pre-wrap-015.html
     53               fast/text/whitespace/pre-wrap-016.html
     54               fast/text/word-break-break-all-010.html
     55               fast/text/word-break-break-all-011.html
     56               fast/text/word-break-break-all-015.html
     57               imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-007.html
     58               imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-008.html
     59               imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-015.html
     60
     61        * rendering/line/BreakingContext.h:
     62        (WebCore::BreakingContext::handleText):
     63
    1642019-03-25  Rob Buis  <rbuis@igalia.com>
    265

  • trunk/Source/WebCore/rendering/line/BreakingContext.h

    r239427 r243437  
    818818        m_currentCharacterIsSpace = c == ' ' || c == '\t' || (!m_preservesNewline && (c == '\n'));
    819819
     820        // A single preserved leading white-space doesn't fulfill the 'betweenWords' condition, however it's indeed a
     821        // soft-breaking opportunty so we may want to avoid breaking in the middle of the word.
     822        if (m_atStart && m_currentCharacterIsSpace && !previousCharacterIsSpace)
     823            breakWords = false;
     824
    820825        if (canHangPunctuationAtStart && m_width.isFirstLine() && !m_width.committedWidth() && !wrapW && !inlineLogicalWidth(m_current.renderer(), true, false)) {
    821826            m_width.addUncommittedWidth(-renderText.hangablePunctuationStartWidth(m_current.offset()));
    … …  
    840845        m_currentCharacterIsWS = m_currentCharacterIsSpace || (breakNBSP && c == noBreakSpace);
    841846
    842         if ((breakAll || breakWords) && !midWordBreak && (!m_currentCharacterIsSpace || style.whiteSpace() != WhiteSpace::PreWrap)) {
     847        if ((breakAll || breakWords) && !midWordBreak && (!m_currentCharacterIsSpace || m_atStart || style.whiteSpace() != WhiteSpace::PreWrap)) {
    843848            wrapW += charWidth;
    844849            bool midWordBreakIsBeforeSurrogatePair = U16_IS_LEAD(c) && U16_IS_TRAIL(renderText.characterAt(m_current.offset() + 1));
Note: See TracChangeset for help on using the changeset viewer.