⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 244748 in webkit


Ignore:
Timestamp:
Apr 29, 2019, 12:56:56 PM (7 years ago)
Author:
jfernandez@igalia.com
Message:

line should not be broken before the first space after a word
​https://bugs.webkit.org/show_bug.cgi?id=197278

Reviewed by Myles C. Maxfield.

LayoutTests/imported/w3c:

Added new Web Platform Tests to cover the cases discussed [1] with the CSS WG.

[1] ​https://github.com/w3c/csswg-drafts/issues/3701

  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013-expected.html: Added.
  • web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013.html: Added.

Source/WebCore:

The 'white-space: break-spaces' only adds breaking opportunities after
a white space character. However, it's possible to break before the
first space after a word when the feature is used in combination with
other properties, like overflow-wrap.

However, breaking before the first space should not be allowed if
there are previous opportunities. We wrongly assumed that we had to
consider these previous breaking opportunities if the proper combination
of line breaking properties is being used, so that breaking before the
first space after a word is allowed.

This wrong assumption caused several issues, like the one described in
the bug, that lead to incorrectly break before the first space even
though there are previous opportunities, either white spaces or between
letters.

Theses issues have been analyzed [1] by the CSS WG and finally agreed on a
expected behavior, represented in the Web Platform tests added in this
patch.

For the later case, of considering previous opportunities between
letters, we have a seperated issue #952254, so the tests covering such
cases will be added to the TestExpecations as Failure entries.

[1] ​https://github.com/w3c/csswg-drafts/issues/3701

Tests: imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001.html

imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012.html
imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013.html

  • rendering/line/BreakingContext.h:

(WebCore::BreakingContext::handleText):
(WebCore::BreakingContext::trailingSpacesHang):

LayoutTests:

Added a few Skip (due to unimplemented features) and Failure (break-all related
issues) entries for some of the tests added by this patch.

Location:
trunk
Files:
26 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r244736 r244748  
     12019-04-29  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        line should not be broken before the first space after a word
     4        https://bugs.webkit.org/show_bug.cgi?id=197278
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        Added a few Skip (due to unimplemented features) and Failure (break-all related
     9        issues) entries for some of the tests added by this patch.
     10
     11        * TestExpectations:
     12
    1132019-04-29  Youenn Fablet  <youenn@apple.com>
    214

  • trunk/LayoutTests/TestExpectations

    r244728 r244748  
    18021802webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/hanging-punctuation/hanging-punctuation-force-end-001.xht [ ImageOnlyFailure ]
    18031803webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/hanging-punctuation/hanging-punctuation-last-001.xht [ ImageOnlyFailure ]
    1804 webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-anywhere-001.html [ ImageOnlyFailure ]
    1805 webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-anywhere-002.html [ ImageOnlyFailure ]
    18061804webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-strict-011.xht [ ImageOnlyFailure ]
    18071805webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-strict-012.xht [ ImageOnlyFailure ]
    … …  
    18811879webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/text-transform/text-transform-upperlower-103.html [ ImageOnlyFailure Pass ]
    18821880webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/text-transform/text-transform-upperlower-104.html [ ImageOnlyFailure Pass ]
    1883 webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-012.html [ ImageOnlyFailure ]
    1884 webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-013.html [ ImageOnlyFailure ]
    18851881webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/textarea-pre-wrap-012.html [ ImageOnlyFailure ]
    18861882webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/textarea-pre-wrap-013.html [ ImageOnlyFailure ]
    … …  
    19501946webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/writing-system/writing-system-line-break-002.html [ ImageOnlyFailure ]
    19511947webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/writing-system/writing-system-text-transform-001.html [ ImageOnlyFailure ]
    1952 webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/writing-system/writing-system-line-break-001.html [ ImageOnlyFailure ]
    19531948webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/hyphens/hyphens-auto-001.html [ ImageOnlyFailure ]
    19541949
    … …  
    19561951webkit.org/b/186045 imported/w3c/web-platform-tests/css/css-animations/animation-delay-009.html [ ImageOnlyFailure Pass ]
    19571952webkit.org/b/186045 imported/w3c/web-platform-tests/css/css-animations/animation-delay-010.html [ ImageOnlyFailure ]
     1953
     1954# The 'line-break: anywhere' feature is not implemented yet
     1955webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-anywhere-001.html [ Skip ]
     1956webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/line-break/line-break-anywhere-002.html [ Skip ]
     1957webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007.html [ Skip ]
     1958webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008.html [ Skip ]
     1959webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009.html [ Skip ]
     1960webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010.html [ Skip ]
     1961webkit.org/b/181169 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011.html [ Skip ]
    19581962
    19591963# overflow-wrap:anywhere feature is not implemented yet.
    … …  
    19711975webkit.org/b/195345 imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-min-content-size-004.html [ Skip ]
    19721976webkit.org/b/195345 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-word-overflow-wrap-interactions.html [ Skip ]
     1977webkit.org/b/195345 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003.html [ Skip ]
     1978webkit.org/b/195345 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006.html [ Skip ]
     1979webkit.org/b/195345 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013.html [ Skip ]
    19731980
    19741981webkit.org/b/197277 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-008.html [ ImageOnlyFailure ]
    1975 
     1982webkit.org/b/197277 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004.html [ ImageOnlyFailure ]
     1983webkit.org/b/197277 imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005.html [ ImageOnlyFailure ]
    19761984
    19771985########################################

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r244728 r244748  
     12019-04-29  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        line should not be broken before the first space after a word
     4        https://bugs.webkit.org/show_bug.cgi?id=197278
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        Added new Web Platform Tests to cover the cases discussed [1] with the CSS WG.
     9
     10        [1] https://github.com/w3c/csswg-drafts/issues/3701
     11
     12        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001-expected.html: Added.
     13        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001.html: Added.
     14        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002-expected.html: Added.
     15        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002.html: Added.
     16        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003-expected.html: Added.
     17        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003.html: Added.
     18        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004-expected.html: Added.
     19        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004.html: Added.
     20        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005-expected.html: Added.
     21        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005.html: Added.
     22        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006-expected.html: Added.
     23        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006.html: Added.
     24        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007-expected.html: Added.
     25        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007.html: Added.
     26        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008-expected.html: Added.
     27        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008.html: Added.
     28        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009-expected.html: Added.
     29        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009.html: Added.
     30        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010-expected.html: Added.
     31        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010.html: Added.
     32        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011-expected.html: Added.
     33        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011.html: Added.
     34        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012-expected.html: Added.
     35        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012.html: Added.
     36        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013-expected.html: Added.
     37        * web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013.html: Added.
     38
    1392019-04-29  Javier Fernandez  <jfernandez@igalia.com>
    240

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r244641 r244748  
    34873487webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-bo-000.html [ ImageOnlyFailure ]
    34883488webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-lo-000.html [ ImageOnlyFailure ]
     3489webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-012.html [ ImageOnlyFailure ]
     3490webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-013.html [ ImageOnlyFailure ]
    34893491
    34903492webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-014.html [ ImageOnlyFailure ]
     3493webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/writing-system/writing-system-line-break-001.html [ ImageOnlyFailure ]
    34913494
    34923495webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/boundary-shaping/boundary-shaping-004.html [ Timeout ImageOnlyFailure ]

  • trunk/LayoutTests/platform/wpe/TestExpectations

    r243930 r244748  
    15171517webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-break-all-005.html [ ImageOnlyFailure ]
    15181518webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/word-break/word-break-normal-bo-000.html [ ImageOnlyFailure ]
     1519webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-012.html [ ImageOnlyFailure ]
     1520webkit.org/b/183258 imported/w3c/web-platform-tests/css/css-text/white-space/pre-wrap-013.html [ ImageOnlyFailure ]
     1521
     1522webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/writing-system/writing-system-line-break-001.html [ ImageOnlyFailure ]
    15191523
    15201524webkit.org/b/195275 imported/w3c/web-platform-tests/css/css-text/boundary-shaping/boundary-shaping-006.html [ Timeout Pass ]

  • trunk/Source/WebCore/ChangeLog

    r244742 r244748  
     12019-04-29  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        line should not be broken before the first space after a word
     4        https://bugs.webkit.org/show_bug.cgi?id=197278
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        The 'white-space: break-spaces' only adds breaking opportunities after
     9        a white space character. However, it's possible to break before the
     10        first space after a word when the feature is used in combination with
     11        other properties, like overflow-wrap.
     12
     13        However, breaking before the first space should not be allowed if
     14        there are previous opportunities. We wrongly assumed that we had to
     15        consider these previous breaking opportunities if the proper combination
     16        of line breaking properties is being used, so that breaking before the
     17        first space after a word is allowed.
     18
     19        This wrong assumption caused several issues, like the one described in
     20        the bug, that lead to incorrectly break before the first space even
     21        though there are previous opportunities, either white spaces or between
     22        letters.
     23
     24        Theses issues have been analyzed [1] by the CSS WG and finally agreed on a
     25        expected behavior, represented in the Web Platform tests added in this
     26        patch.
     27
     28        For the later case, of considering previous opportunities between
     29        letters, we have a seperated issue #952254, so the tests covering such
     30        cases will be added to the TestExpecations as Failure entries.
     31
     32        [1] https://github.com/w3c/csswg-drafts/issues/3701
     33
     34        Tests: imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-001.html
     35               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-002.html
     36               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-003.html
     37               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-004.html
     38               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-005.html
     39               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-006.html
     40               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-007.html
     41               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-008.html
     42               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-009.html
     43               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-010.html
     44               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-011.html
     45               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-012.html
     46               imported/w3c/web-platform-tests/css/css-text/white-space/break-spaces-before-first-char-013.html
     47
     48        * rendering/line/BreakingContext.h:
     49        (WebCore::BreakingContext::handleText):
     50        (WebCore::BreakingContext::trailingSpacesHang):
     51
    1522019-04-29  Chris Dumez  <cdumez@apple.com>
    253

  • trunk/Source/WebCore/rendering/SimpleLineLayout.cpp

    r244036 r244748  
    822822                    break;
    823823                }
    824                 if (style.breakSpaces && style.breakWordOnOverflow && line.hasWhitespaceFragments() && fragment.length() == 1) {
     824                if (style.breakSpaces && line.hasWhitespaceFragments() && fragment.length() == 1) {
    825825                    // Breaking before the first space after a word is not allowed if there are previous breaking opportunities in the line.
    826826                    textFragmentIterator.revertToEndOfFragment(line.revertToLastCompleteFragment(runs));

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

    r244036 r244748  
    756756        // soft-breaking opportunty so we may want to avoid breaking in the middle of the word.
    757757        if (m_atStart && m_currentCharacterIsSpace && !previousCharacterIsSpace) {
    758             m_hasFormerOpportunity = canBreakMidWord;
     758            m_hasFormerOpportunity = true;
    759759            breakWords = false;
    760760            canBreakMidWord = breakAll;
    … …  
    954954                // Auto-wrapping text should not wrap in the middle of a word once it has had an
    955955                // opportunity to break after a word.
    956                 m_hasFormerOpportunity = canBreakMidWord;
     956                m_hasFormerOpportunity = true;
    957957                breakWords = false;
    958958                canBreakMidWord = breakAll;
    … …  
    10921092    // Avoid breaking before the first white-space after a word if there is a
    10931093    // breaking opportunity before.
    1094     if (m_hasFormerOpportunity)
     1094    if (m_hasFormerOpportunity && !previousCharacterIsSpace)
    10951095        return;
    10961096
Note: See TracChangeset for help on using the changeset viewer.