188759 - right-margin problems of justified Arabic text (all fonts)

Status: RESOLVED FIXED

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

Description

[Thamer Mahmoud]

This happened after bug 187866 was fixed and it's not reproducible in 
build 20030106 (or any post bug 131023 build)

TO REPRODUCE:

1. Go to URL
2. Ctrl+A to select all text (just for clarity)
3. look at the third paragraph


ACTUAL: miss-aligned right margins on some lines. 
EXPECTED: even margins

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->smontagu

Comment 2

[Jason Bassford]

WFM.  2003011204 / XP.  The lines are all evenly right-aligned.  Unless I don't
understand the report.

Comment 3

[Andreas Kunz]

The same for me. (2003011204 on Win2k)
You mean it worked from December until a few days ago?
What build are you using? Did the page change?

Comment 4

[Thamer Mahmoud]

Attachment: Screenshot

Here is a screenshot of the problem on build 2003011208.

Comment 5

[Jason Bassford]

Still WFM with the 1208 build under XP.  I do not see what's shown in the
screenshot - everything lines up evenly at the right.

Reporter: Did you do a completely clean install of Mozilla and create a new
profile to test this with?

Comment 6

[Simon Montagu :smontagu]

Attachment: Reduced testcase

On Win2K I can always reproduce the bug with this testcase. If the error
doesn't appear on initial load, it can be made to appear by making the window
wider or narrower. It seems to be always in the line containing the word ÃáÇ
(alef hamza, lamalef shadda).

If the bug doesn't appear on XP, it's possibly a bug in the measuring routines
in Windows which was corrected between 2000 and XP.

Comment 7

[Simon Montagu :smontagu]

Attachment: Even more reduced testcase

Both lines should look the same, with the whole word appearing on a yellow
background

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot with my current trunk build - seems to work for me.

Comment 9

[Simon Montagu :smontagu]

Attachment: Screenshot on XP with Arabic support installed

OK, the difference is not between W2K and XP but between Windows systems with
and without Arabic support installed. This screenshot is from XP with "support
for complex scripts and right-to-left languages" installed and is the same as
on my W2K machine.

Comment 10

[Simon Montagu :smontagu]

The nearest to a correct rendering, by the way, is the top example in attachment
113818.

Comment 11

[Simon Montagu :smontagu]

Both text frames in the attachment are given the correct width in
nsTextFrame::Reflow, but by the time we get to nsTextFrame::Paint, the
mRect.width of the justified frame is too small. What could be changing it?

Comment 12

[Simon Montagu :smontagu]

My last comment was not very accurate. The justified frame is measured
incorrectly because of a bug in nsTextTransformer::GetNextWord. After shaping
the Arabic text and converting lam + alef to a lam alef ligature (or any other
transformation which makes the output length different from the input length),
we should be resetting the pointer to the transformed buffer. 

Most of the time this was resulting mismeasurement was imperceptible (since we
were measuring the correct number of characters, though not the same characters
that we were rendering), but in the case here of a word containing a lam + alef
and a diacritic with zero width, there was a noticeable difference.

Comment 13

[Simon Montagu :smontagu]

Attachment: Patch

Comment 14

[rbs]

Comment on attachment 114055 [details] [diff] [review]
Patch

+   mBufferPos = prevBufferPos + *aWordLenResult;

Actually, this should be an invariant, right? i.e., there should be an
post-condition in that function: 

NS_ASSERTION(mBufferPos == prevBufferPos + *aWordLenResult, "internal error");

Comment 15

[rbs]

Comment on attachment 114055 [details] [diff] [review]
Patch

Tested and didn't see any secondary weirdness. Please document this general
invariant with the suggested assertion.

Comment 16

[Simon Montagu :smontagu]

Fix checked in.