110497 - ZWNJ and ZWJ ignored in Arabic context

Status: RESOLVED FIXED

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

Description

[Roozbeh Pournader]

In the nightly build 2001111603, under Windows 2000, U+200C, Zero Width
Non-Joiner is ignored. The character should break cursive joining of adjacent
Arabic letters.
Mozilla's behaviour was OK in 0.9.5, so this is introduced in the meanwhile.

Comment 1

[Roozbeh Pournader]

Attachment: Correct behaviour from 0.9.5

Comment 2

[Roozbeh Pournader]

Attachment: Buggy behaviour from 2001111603

Comment 3

[Simon Montagu :smontagu]

Lina, was this regression caused by the fix to bug 36163?

Comment 4

[Lina Kemmel]

Dear Simon, sorry about the late reply - I've seen your question only today..
My first guess was that the fix to bug 36163 could definitely cause this regression. However, Zero Width
Non-Joiner, CH_ZWNJ, was defined as discardable also before my changes (see nsTextTransformer.h). To see 
whether my changes cause the problem or not, it probably makes sense to remove ZWNJ from the definition 
of IS_BIDI_CONTROL(_ch), agree? And btw, is Zero Width *Joiner* ignored?

Comment 5

[Roozbeh Pournader]

Yes Lina, Zero Width Joiner is also ignored in the latest nightly I am testing
with (2001121703).

Comment 6

[Simon Montagu :smontagu]

OK, as Lina suggested I removed ZWNJ and ZWJ from the definition of
IS_BIDI_CONTROL. This corrected the shaping and regressed bug 36163 for those
characters. Here is the diff for reference:

diff -u -r1.24 nsTextTransformer.h
--- layout/html/base/src/nsTextTransformer.h    14 Nov 2001 14:21:52 -0000    
+++ layout/html/base/src/nsTextTransformer.h    19 Dec 2001 18:37:55 -0000
@@ -65,7 +65,7 @@
 #define CH_RLO  8238  //<!CDATA "&#8238;" -- right-to-left override, U+202E-->
 #define IS_BIDI_CONTROL(_ch) \
-  (((_ch) >= CH_ZWNJ && (_ch) <= CH_RLM) \
+  (((_ch) >= CH_LRM && (_ch) <= CH_RLM) \
   || ((_ch) >= CH_LRE && (_ch) <= CH_RLO))
 #endif // IBMBIDI

I am changing the dependency from meta bug 115713 to meta bug 115705. Even if
this isn't really a measurement issue, the solution for the measurement problem
needs to take this problem into account.

Comment 7

[Simon Montagu :smontagu]

*** Bug 117281 has been marked as a duplicate of this bug. ***

Comment 8

[Frank Tang]

smontagu:
what should we do about the change you suggest in nsTextTransform.h ?
will that solve the problem ? any bad side effect? should we ask r= and check in ?

Comment 9

[Simon Montagu :smontagu]

We can't check in that change in isolation, because it regresses bug 36163.
Depending on how we solve the problems with measurement of Arabic shaped text,
we may have to check in that patch with the additional of more changes to strip
the zwnj and zwj characters after shaping.

Having written that, I realize that the dependencies in this bug are wrong. It
depends on 115705, rather than blocking it.

Comment 10

[Simon Montagu :smontagu]

Attachment: Patch

This patch has been taken out of bug 99823 in order to separate issues.

Comment 11

[Simon Montagu :smontagu]

I often can't connect to www.persianacademy.ir, but there is a test case in
attachment 62995 [details] from the dupe bug 117281

Comment 12

[Simon Montagu :smontagu]

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

Ignore this patch - it doesn't work.

Comment 13

[Simon Montagu :smontagu]

Attachment: Patch v.2

Ok, to fix this I have had to bring StripBidiControlCharacters back from CVS
limbo. This is temporary, and it can be banished to the outer darkness again
once bug 99823 and its dependencies are fixed.

Comment 14

[Frank Tang]

Comment on attachment 79733 [details] [diff] [review]
Patch v.2

r=ftang

Comment 15

[Marc Attinasi]

Comment on attachment 79733 [details] [diff] [review]
Patch v.2

sr=attinasi

Comment 16

[Simon Montagu :smontagu]

Fix checked in