Justify space should be added both side of justifiable char

RESOLVED DUPLICATE of bug 1063857

Status

()

defect
RESOLVED DUPLICATE of bug 1063857
15 years ago
5 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Depends on 1 bug, {intl})

Trunk
mozilla1.9alpha1
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: almost fixed on new text frame, but we need more work, see comment 16, )

Attachments

(7 obsolete attachments)

Summary: On CJ justification, the previous charachter of CJ justifiable charachter should be justifiable → On CJ justification, the previous character of CJ justifiable character should be justifiable
Posted patch Patch rv1.0 (obsolete) — Splinter Review
Attachment #179278 - Attachment is obsolete: true
Attachment #179469 - Flags: superreview?(roc)
Attachment #179469 - Flags: review?(roc)
I added the "Extra Justifiable Character" in the patch.
That means that if the character is single, it is not a justifiable character.
but if the character is before *CJ* justifiable character, it becomes extra
justifiable character.

For example.
<p lang="ja"><span lang="en>a bc</span> def</p>

"c" is before the space that is justifiable character and is Japanese lang.
In this time, "c" is Extra Justifiable Character.
"a" is before the space. But this space is justifiable character and is *not*
Japanese lang and Chinese lang(that is english).
In this time, "a" is not Extra Justifiable Character.
Could we avoid this problem another way ... by putting half the justification
space for a character on one side of the glyph, and half on the other side?
Robert:

I should change gfx on All platforms?
e.g., On Windows
http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsRenderingContextWin.cpp#2207

2233       x = data->mX;                                    <---- Here?
2234       y = data->mY;
2235       data->mTranMatrix->TransformCoord(&x, &y);
2236       if (IS_HIGH_SURROGATE(*str) && 
2237           ((str+1)<end) && 
2238           IS_LOW_SURROGATE(*(str+1))) 
2239       {
2240         // special case for surrogate pair
2241         fontWin->DrawString(data->mDC, x, y, str, 2);
2242         // we need to advance data->mX and str twice
2243         data->mX += *data->mSpacing++;
2244         ++str;
2245       } else {
2246         fontWin->DrawString(data->mDC, x, y, str, 1);
2247       }
2248       data->mX += *data->mSpacing++;
2249       ++str;
Oops..

Comment 5 is wrong way.
In this case, the first character of the line is not rendered correctly.
Umm... I should add the parameter "const nscoord* aPreSpacing" for |DrawString|?
I think that if we can fix this bug, we can implement text-autospace property in
bug 289130.
Blocks: 289130
Summary: On CJ justification, the previous character of CJ justifiable character should be justifiable → Justify space should be added both side of justifiable char
Posted patch Patch rv2.1 (obsolete) — Splinter Review
fix the bustage on Linux with GTK.
Attachment #182956 - Attachment is obsolete: true
Attachment #182973 - Flags: superreview?(roc)
Attachment #182973 - Flags: review?(roc)
Posted patch Patch rv2.2 (obsolete) — Splinter Review
fix the bustage of cairo on Linux.

I tested on following toolkit and OS.
Win32, GTK without Xft, GTK2 with Xft, GTK2 with cairo and xlib.
Attachment #182973 - Attachment is obsolete: true
Attachment #183088 - Flags: superreview?(roc)
Attachment #183088 - Flags: review?(roc)
Roc:

I want your comment that the patch's approach is good or not good.
If you grant the approach, I will go to next step(test and separate the patch by
reviewer).
Posted patch Patch rv2.4 (obsolete) — Splinter Review
fix the bug of pango.
Attachment #183183 - Attachment is obsolete: true
Attachment #183404 - Flags: superreview?(roc)
Attachment #183404 - Flags: review?(roc)
I don't know why the aCharOffset array is needed. It seems like you could just
add extra space to the aSpacing array and adjust the initial aX parameter.

Furthermore, I wonder if the approach of associating justification space with
glyphs is correct. For example, if we have two CJK characters "J" and one
non-CJK letter "R", then given
RJJR
should we really have twice as much space between "JJ" as between "RJ"? Maybe a
better approach would be to identify points between glyphs where justification
space can be inserted, and evenly distribute the space among those points.

Masayuki, I think we should put this bug off until 1.9, and probably until we
have the new Gfx layer (i.e., mostly cairo only). Then this kind of thing will
be much easier to fix (and much less risky). Perhaps we can work out a better
way to handle all this processing. For example, we could have an API that asks
the native i18n font layer (Pango or Uniscribe or ATSUI) to convert the Unicode
text into a string of glyphs with information about each glyph (the glyph's
position, and whether space can be inserted before or after the glyph for
letter-spacing, word-spacing, justification and even line-breaking). Then we
could process the glyph string, repositioning glyphs according to the spacing.
(In reply to comment #16)
> Perhaps we can work out a better
> way to handle all this processing. For example, we could have an API that asks
> the native i18n font layer (Pango or Uniscribe or ATSUI) to convert the Unicode
> text into a string of glyphs with information about each glyph (the glyph's
> position, and whether space can be inserted before or after the glyph for
> letter-spacing, word-spacing, justification and even line-breaking). Then we
> could process the glyph string, repositioning glyphs according to the spacing.

FYI, I'm working on something like this at the moment, which I hope will be
ready for the beginning of the 1.9 cycle. I don't see any other way to handle
justification of text with ligatures and combining characters.
> Maybe a
> better approach would be to identify points between glyphs where justification
> space can be inserted, and evenly distribute the space among those points.

It is first approach. In the way, we need next character always.(I.e., if a
character is end of text frame, we should get a first character of next text frame.)

> I think we should put this bug off until 1.9,

I agree it. I have targeted this to 1.9a.

> and probably until we
> have the new Gfx layer (i.e., mostly cairo only).

O.K.
Attachment #183404 - Attachment is obsolete: true
Attachment #183404 - Flags: superreview?(roc)
Attachment #183404 - Flags: review?(roc)
fantasa and Ian:

I'll work on this for better justify implementation after nsTextFrame is refactored. But I have a question for the spec, CSS2.1 said:

http://www.w3.org/TR/CSS21/text.html#propdef-letter-spacing
> User agents may not further increase or decrease the inter-character space in order to justify text.

That meanes we cannot add the extra space to any character for the justification if the element has non-zero letter-spacing. This is bad spec for intl justification(i.e., for Japanese and Chinese). And we are adding the extra space to the SPACE(U+0020) and NBSP(U+00A0) if the element has non-zero letter-spacing. So, if the element has non-zero letter-spacing, we cannot add extra space for justification on every character...

Now, Gecko uses the letter-spacing value as minimum inter-character space in justify layout. I think that this behavior is better than spec. What do you think about this?
CSS3 Text will introduce min/max controls on letter-spacing [1], so there will be finer control over this aspect in CSS3. Disallowing inter-character justification when letter-spacing is a <length> has been specced since CSS1, so it is unlikely to change.. Although I do agree that it makes more sense to allow it, especially in non-Latin contexts.

I have not seen Mozilla do inter-character justification before. What would trigger it?

[1] http://www.w3.org/TR/css3-text/#letter-spacing
(In reply to comment #21)
> I have not seen Mozilla do inter-character justification before. What would
> trigger it?

I have implemented justification for Japanese and Chinese. In these cases, extra spaces for justification are added between ideograph characters. See bug 36322.
should fix only on new text frame.
# it already fixes the almost of this bug, but I need a little work (comment 16).
Depends on: 333659
No longer depends on: 295483, 297074
Whiteboard: almost fixed on new text frame, but we need more work, see comment 16
This was fixed by a fix of bug 1063857.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1063857
You need to log in before you can comment on or make changes to this bug.