Closed Bug 288439 Opened 20 years ago Closed 10 years ago

Justify space should be added both side of justifiable char

Categories

(Core :: Layout: Text and Fonts, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1063857
mozilla1.9alpha1

People

(Reporter: masayuki, Assigned: masayuki)

References

()

Details

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

Attachments

(7 obsolete files)

On CJ justification layout, the previous charachter of CJ justifiable charachter should be justifiable. testcase1 http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2657&action=view screenshot of textcase1 http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2658&action=view testcase2 http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2659&action=view
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
Attached patch Patch rv0.1(work in progress) (obsolete) (deleted) — Splinter Review
Attached patch Patch rv1.0 (obsolete) (deleted) — Splinter Review
Attachment #179278 - Attachment is obsolete: true
Attachment #179469 - Flags: superreview?(roc)
Attachment #179469 - Flags: review?(roc)
Status: NEW → ASSIGNED
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|?
Attachment #179469 - Flags: superreview?(roc)
Attachment #179469 - Flags: review?(roc)
Attachment #179469 - Flags: review-
I think that if we can fix this bug, we can implement text-autospace property in bug 289130.
Blocks: 289130
Target Milestone: --- → mozilla1.9alpha
Summary: On CJ justification, the previous character of CJ justifiable character should be justifiable → Justify space should be added both side of justifiable char
Attached patch Patch rv2.0 (obsolete) (deleted) — Splinter Review
Attachment #179469 - Attachment is obsolete: true
Comment on attachment 182956 [details] [diff] [review] Patch rv2.0 How about this?
Attachment #182956 - Flags: superreview?(roc)
Attachment #182956 - Flags: review?(roc)
Attachment #182956 - Flags: superreview?(roc)
Attachment #182956 - Flags: review?(roc)
Attached patch Patch rv2.1 (obsolete) (deleted) — 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)
Attachment #182973 - Flags: superreview?(roc)
Attachment #182973 - Flags: review?(roc)
Attached patch Patch rv2.2 (obsolete) (deleted) — 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)
Attached patch Patch rv2.3 (obsolete) (deleted) — Splinter Review
Attachment #183088 - Flags: superreview?(roc)
Attachment #183088 - Flags: review?(roc)
Comment on attachment 183183 [details] [diff] [review] Patch rv2.3 I tested Qt too.
Attachment #183183 - Flags: superreview?(roc)
Attachment #183183 - Flags: review?(roc)
Attachment #183088 - Attachment is obsolete: true
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).
Attachment #183183 - Flags: superreview?(roc)
Attachment #183183 - Flags: review?(roc)
Attached patch Patch rv2.4 (obsolete) (deleted) — 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.
No longer blocks: 289130
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: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: