Bug 17705

Summary: Whitespace between nowrap elements ignored after collapsed trailing space in a text run
Product: WebKit Reporter: Kier Darby <kier>
Component: Layout and RenderingAssignee: Robert Hogan <robert>
Status: RESOLVED FIXED    
Severity: Normal CC: aravind.akella, eric, esprehn+autocc, leviw, mitz, ojan.autocc, robert, webkit.review.bot
Priority: P3 Keywords: HasReduction
Version: 523.x (Safari 3)   
Hardware: All   
OS: All   
URL: http://kier.vbulletin.com/testcases/safari-whitespace.html
Attachments:
Description Flags
Test case showing incorrect behavior
none
Patch
none
Patch none

Description Kier Darby 2008-03-07 08:01:50 PST
When placing two <span> elements with white-space:nowrap next to each other but separated by whitespace, under some conditions the whitespace is ignored and the two <span> elements will not wrap together when space is constrained.

A test case is available, showing how minor changes can change this behavior.
Comment 1 Kier Darby 2008-03-07 08:03:41 PST
Created attachment 19585 [details]
Test case showing incorrect behavior
Comment 2 aravind.akella 2010-11-02 12:39:16 PDT
In all the three cases, both the spans should be on separate lines .... rite ? Thats how firefox and IE display this file. Case 2 should be incorrect as well ?
Comment 3 Robert Hogan 2013-03-05 13:02:06 PST
Created attachment 191544 [details]
Patch
Comment 4 Ryosuke Niwa 2013-04-09 12:20:39 PDT
Comment on attachment 191544 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:3081
> +                    // If we allow whitespace collapsing 'word  ' and 'word' are equivalent when before a whitespace
> +                    // character, so treat both as a potential linebreak.

Nit: I would insert a comma after "If we allow whitespace collapsing" and get rid of "when" in "when before a whitespace".

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:3082
> +                    checkForBreak = (ignoringSpaces || !currentCharacterIsSpace) && (c == ' ' || c == '\t' || (c == '\n' && !next->preservesNewline()));

We should extract c == ' ' || c == '\t' || (c == '\n' && !next->preservesNewline() as a helper function at some point.

> LayoutTests/fast/text/whitespace/inline-whitespace-wrapping-5.html:20
> +    <span class="nowrap">xx  </span> 
> +    <span class="nowrap">xx</span>

Please add a test case for bidirectional text.
Comment 5 Robert Hogan 2013-04-13 09:42:26 PDT
Created attachment 197947 [details]
Patch
Comment 6 Robert Hogan 2013-04-13 15:01:14 PDT
Committed r148367: <http://trac.webkit.org/changeset/148367>
Comment 7 Alexey Proskuryakov 2013-06-07 18:05:06 PDT
This caused bug 117284.