Bug 69072 - White space between inline blocks should be affected by word-spacing property
: White space between inline blocks should be affected by word-spacing property
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-09-29 01:43 PST by
Modified: 2012-12-12 11:44 PST (History)


Attachments
white-space processing test (983 bytes, text/html)
2011-09-29 01:43 PST, Lev Solntsev
no flags Details
Patch (16.13 KB, patch)
2012-06-23 08:05 PST, Robert Hogan
no flags Review Patch | Details | Formatted Diff | Diff
Patch (11.92 KB, patch)
2012-06-25 14:56 PST, Robert Hogan
no flags Review Patch | Details | Formatted Diff | Diff
Patch (12.61 KB, patch)
2012-12-10 13:18 PST, Robert Hogan
no flags Review Patch | Details | Formatted Diff | Diff
Patch (12.38 KB, patch)
2012-12-10 13:33 PST, Robert Hogan
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-09-29 01:43:09 PST
Created an attachment (id=109135) [details]
white-space processing test

According to fantasai message in www-style:
> http://www.w3.org/TR/CSS21/text.html#spacing-props
>   # Word spacing affects each space (U+0020) and non-breaking space (U+00A0),
>   # left in the text after the white space processing rules have been applied.
>  
> This means that
>  
> <inline-block/><inline-block/> will only be affected by letter-spacing, whereas
> <inline-block/> <inline-block/> will be affected by word-spacing.http://lists.w3.org/Archives/Public/www-style/2011Sep/0493.html

For now word-spacing property in Webkit doesn't affect white space between
inline blocks but does between usual words. Instead white space between
inline-blocks is affected by letter-spacing property.

Word-spacing property is widely used to eliminate spaces between inline-blocks
produced by code formatting white-spaces.
------- Comment #1 From 2012-03-11 07:11:24 PST -------
It looks like the place to do this with RenderStyle::wordSpacing() is in RenderBlock::computeInlinePreferredLogicalWidths()
------- Comment #2 From 2012-06-05 08:57:27 PST -------
This seems to be similar to this Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=761645

Simple example of the bug here: http://jon.thesquareplanet.com/inline-block-bug/
------- Comment #3 From 2012-06-23 08:05:56 PST -------
Created an attachment (id=149175) [details]
Patch
------- Comment #4 From 2012-06-25 14:56:18 PST -------
Created an attachment (id=149363) [details]
Patch
------- Comment #5 From 2012-06-25 15:00:57 PST -------
Robert Hogan, probably you should change the authorship meta information in tests since you largely reworked those.
------- Comment #6 From 2012-07-02 11:50:01 PST -------
Hi Dan,

Could you take a look at this please?

Thanks,
Robert
------- Comment #7 From 2012-12-10 12:29:41 PST -------
(From update of attachment 149363 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=149363&action=review

> Source/WebCore/rendering/InlineFlowBox.cpp:428
> +                // If we encounter any space after this inline block then ensure it is treated as the space between two words.
> +                needsWordSpacing = true;

Why don't we have to account for this in RenderBlock::computeInlinePreferredLogicalWidths?

> LayoutTests/fast/css/word-spacing-between-blocks-expected.html:9
> +    <link rel="author" title="Lev Solntsev" href="mailto:grelimail@gmail.com">
> +    <link rel="author" title="Robert Hogan">

You may want to address Lev's concerns.

> LayoutTests/fast/css/word-spacing-between-blocks-expected.html:28
> +<p>Following strings must be same:</p>

Since these are reworked, please give these tests a meaningful description and a link back to this bug.
------- Comment #8 From 2012-12-10 13:09:19 PST -------
(From update of attachment 149363 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=149363&action=review

>> Source/WebCore/rendering/InlineFlowBox.cpp:428
>> +                needsWordSpacing = true;
> 
> Why don't we have to account for this in RenderBlock::computeInlinePreferredLogicalWidths?

There, the word-spacing between inline blocks is account for by:

t->trimmedPrefWidths(inlineMax, beginMin, beginWS, endMin, endWS,
                                     hasBreakableChar, hasBreak, beginMax, endMax,
                                     childMin, childMax, stripFrontSpaces);
------- Comment #9 From 2012-12-10 13:18:22 PST -------
Created an attachment (id=178617) [details]
Patch
------- Comment #10 From 2012-12-10 13:20:24 PST -------
(From update of attachment 178617 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=178617&action=review

> LayoutTests/fast/css/word-spacing-between-blocks-expected.html:30
> +<!-- White space between inline blocks should be affected by the word-spacing property.
> +     https://bugs.webkit.org/show_bug.cgi?id=69072 -->

Is there a reason you made this a comment instead of allowing the description in the output?
------- Comment #11 From 2012-12-10 13:21:40 PST -------
(In reply to comment #8)
> (From update of attachment 149363 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=149363&action=review
> 
> >> Source/WebCore/rendering/InlineFlowBox.cpp:428
> >> +                needsWordSpacing = true;
> > 
> > Why don't we have to account for this in RenderBlock::computeInlinePreferredLogicalWidths?
> 
> There, the word-spacing between inline blocks is account for by:
> 
> t->trimmedPrefWidths(inlineMax, beginMin, beginWS, endMin, endWS,
>                                      hasBreakableChar, hasBreak, beginMax, endMax,
>                                      childMin, childMax, stripFrontSpaces);

So beyond just failing to follow the intention of the spec, we're currently deciding to respect word-spacing for inline blocks *differently* during pref-width-calc and layout? Sadness :(
------- Comment #12 From 2012-12-10 13:33:08 PST -------
Created an attachment (id=178622) [details]
Patch
------- Comment #13 From 2012-12-12 11:44:47 PST -------
(From update of attachment 178622 [details])
Clearing flags on attachment: 178622

Committed r137494: <http://trac.webkit.org/changeset/137494>
------- Comment #14 From 2012-12-12 11:44:51 PST -------
All reviewed patches have been landed.  Closing bug.