Re: CSS3 Text patch review as of 2010/12/12

On 12/11/2010 10:43 AM, Koji Ishii wrote:
> I'm not sure how much spare time you have these days, but the diff is getting bigger
> and I'm afraid you may end up reviewing the same one multiple times, so sending the
> patch until now after reverted/fixed all the issues pointed out.
>
> I'll keep editing anyway, so if you couldn't have time until I send newer diff, it's
> fine to disregard this.

Thank you for working on this and keeping up with the feedback. :)
When you check in, please include a www-style archive link for the
messages you are responding to in your checkin comment. (If it's a
thread, pick a representative message -- either one describing the
issue, or one describing the agreed solution.) And of course send
a response to the commenter with a pointer to the resulting changes.

> CHANGES:
> * text-transform accepts multiple values (www-style feedback)
> * Some syntax (the use of '' and ') fixes
> * Added text-justify:none (www-style feedback)
> * Changed text-justify examples from 4051 to JLREQ
> * Add description to text-justify:distribute for when single char (ISSUE-97)
> * Add description to punctuation-trim for when kerning data exsists
> * text-autospace describes behavior for element boundaries
> * text-autospace describes exception cases (sent to www-style)
> * Changed how text-emphasis should be rendered to be more UA dependent
> * Added Ken's font as an example for text-emphasis
>
> NOTES:
> * The proposed diff where Murakami-san objected was reverted. Murakami-san seems to
>   be very strong about that, and it turned out that it's not an urgent issue anyway.
> * The proposed diff for text-emphasis line height issue was reverted. I agree that
>   it's better to improve ruby spec.

Here is my review of the changes. My comments are mostly editorial,
aside from the fallback behavior you are suggesting for 'distribute'--
I think we should use 'text-align-last'. See below.

> -<td>none | capitalize | uppercase | lowercase | fullwidth | large-kana
> -</td>
> +<td>none |
> +            [ [ capitalize | uppercase | lowercase ] || fullwidth || large-kana ]</td>

r+

> -<li>Otherwise, if ''text-autospace'' property is set to add extra spaces
> +<li>Otherwise, if 'text-autospace' property is set to add extra spaces

r+

> -<li>

r+

> +            none |
>               [ trim || [ inter-word | inter-ideograph | inter-cluster | distribute | kashida ] ]
>  ...
> +<dt><dfn title="text-justify:none"><code>none</code></dfn></dt>
> +<dd>Justification is disabled.
> +<span class="note">This value is intended for use in user stylesheets
> +          to improve readability or for accessibility purposes.</span></dd>

r+

>           <dd>Justification primarily changes spacing at word separators and
>             at inter-graphemic boundaries in scripts that use no word spaces.
>             This value is typically used for<abbr title="Chinese/Japanse/Korean">CJK</abbr>
> -          languages.</dd>
> +          languages.
> +<span class="note">
> +          An example of justification rules for Japanese is defined by
> +          3.8 Line Adjustment in the [[!JLREQ]].</span>
> +</dd>

I think this example should go in the more general prose material for justification,
not just to this one value. It could, for example, also apply to ''auto''. And IIRC,
JLREQ's behavior incorporates aspects of ''trim'' and also interacts with
'punctuation-trim'.
>           <dd>Justification primarily changes spacing both at word separators
>             and at grapheme cluster boundaries in all scripts except those in
>             the connected and cursive groups.
> +          If there are no word separators nor grapheme cluster boundaries,
> +          the inline contents are centered within the line box.

The behavior when there are no expansion opportunities in a line
is to fall back to the value given in text-align-last. I don't think
it makes sense to have 'distribute' be an exception here.

What we might want to do, though, is have the 'justify' value of
'text-align-last' fall back to 'center' instead of to 'start'.
And/or allow 'text-align-last' to combine 'justify' with another
value.

> +<span class="note">
> +          An example of compression rules for Japanese is defined by
> +          3.8 Line Adjustment in the [[!JLREQ]].</span>
> +<p class="issue">The rule a. of 3.8.3 in the current JLREQ may produce
> +          non-optimal results when the rules are applied to English lines,
> +          or sometimes English words within Japanese lines.
> +          There's a discussion currently going on for how to manage this.</p>

I would write that as
   "An example of compression rules is given for Japanese in..."

Otherwise r+.

>       <p>Fullwidth opening and closing punctuation must not be trimmed if the
>         glyph is not actually fullwidth. A fullwidth glyph is one that has the
> -      same advance width as a typical Han character in the same font.</p>
> +      same advance width as a typical Han character in the same font.
> +      This includes the case where the glyph is not fullwidth
> +      as a result of the kerning in the font.</p>

I would swap the order of the last two sentences so that the one you're adding
connects more closely with the firs sentence (the case it's modifying).

>       <p>Whether fullwidth colon punctuation and fullwidth dot punctuation should
>         be considered fullwidth closing punctuation or fullwidth middle dot
> @@ -2195,10 +2180,18 @@
>         <dt><dfn title="text-autospace:none"><code>none</code></dfn></dt>
>           <dd>No extra space is created.</dd>
>         <dt><dfn title="text-autospace:ideograph-numeric"><code>ideograph-numeric</code></dfn></dt>
> -<dd>Creates 1/4em extra spacing between runs of ideographic letters and numeric glyphs.</dd>
> +<dd>Creates 1/4em extra spacing between runs of ideographic letters and numeric glyphs.
> +          At element boundaries, the em is given by and rendered within the innermost element that contains the boundary.
> +          The extra spacing should not be created if the numeric glyphs are fullwidth,
> +          upright in vertical text flow using the 'text-orientation' property or the 'text-combine' property,
> +          or have different computed value of the 'writing-mode' property than the ideographic letters.</dd>
>         <dt><dfn title="text-autospace:ideograph-alpha"><code>ideograph-alpha</code></dfn></dt>
>           <dd>Creates 1/4em extra spacing between runs of ideographic letters and
> -          non-ideographic letters, such as Latin-based, Cyrillic, Greek, Arabic or Hebrew.</dd>
> +          non-ideographic letters, such as Latin-based, Cyrillic, Greek, Arabic or Hebrew.
> +          At element boundaries, the em is given by and rendered within the innermost element that contains the boundary.
> +          The extra spacing should not be created if the non-ideographic letters are fullwidth,
> +          upright in vertical text flow using the 'text-orientation' property or the 'text-combine' property,
> +          or have different computed value of the 'writing-mode' property than the ideographic letters.</dd>

I would add these sentences in the general prose rather than to specific values,
since they apply to the property generally and are not specific to particular
values.

>       <p>The marks should be drawn using the element's font settings with its
> -      size scaled down to 50%. UA should fall back to an appropriate font if
> -      the glyph is missing in the font. The marks may instead be synthesized
> -      by the UA.</p>
> -
> -<p class="issue">This rendering scheme is based on ruby algorithm and the one used in Japanese printing industries.
> -      But the size of glyphs varies so much that this may result in inconsistent and/or bad visuals.
> -      If you have any feedback on this, it's appreciated.</p>
> +      size scaled down to 50%.
> +      However, not all fonts have all these glyphs,
> +      and some fonts use inappropriate sizes for emphasis marks in these code points.
> +      The UA may opt to use a font known to be good for emphasis marks,
> +      or the marks may instead be synthesized by the UA.</p>
> +
> +<div class="example">
> +<p>One example of good fonts for emphasis marks is Adobe's opensource project,
> +<a href="http://sourceforge.net/adobe/kenten-generic/">Kenten Generic OpenType Font</a>,
> +        which is specially designed for the emphasis marks.</p>
> +</div>

r+

CC'ed www-archive for message archival.

~fantasai

Received on Monday, 13 December 2010 22:47:01 UTC