[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    [PATCH] [Bug 91955] Bidi algorithm: incorrect implementation of
From:       Mitz Pettel <bugs.kde.org () mitzpettel ! com>
Date:       2004-11-02 23:17:13
Message-ID: 540A3996-2D25-11D9-96AD-0003937C3DB0 () mitzpettel ! com
[Download RAW message or body]

Hi,

Stephan Kulow has suggested that I post here.

Attached is a proposed patch for kdelibs/khtml/rendering/bidi.cpp to 
resolve bug 91955 (incorrect implementation of rule N2 of the Unicode 
bidi algorithm).

I cleaned up the code at two places in bidiReorderLine, then I 
corrected the issue in the dirCurrent==[AL|R], 
bidi.status.last==[ET|ES|CS|BN|B|S|WS|ON] case, then I adapted the same 
logic to the dirCurrent==L, bidi.status.last==[ET|ES|CS|BN|B|S|WS|ON] 
case. Also set dir and status.eor after the appendRun in the 
dirCurrent==L, bidi.status.last==[R|AL|EN|AN] case, for consistency and 
clarity.


["bidi.cpp.diff" (bidi.cpp.diff)]

Index: bidi.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/rendering/bidi.cpp,v
retrieving revision 1.198
diff -u -r1.198 bidi.cpp
--- bidi.cpp	26 Oct 2004 18:58:41 -0000	1.198
+++ bidi.cpp	2 Nov 2004 22:41:33 -0000
@@ -856,26 +856,19 @@
                 case QChar::DirS:
                 case QChar::DirWS:
                 case QChar::DirON:
-                    if(dir != QChar::DirL) {
+                     if( bidi.status.eor != QChar::DirL ) {
                         //last stuff takes embedding dir
-                        if( bidi.context->dir == QChar::DirR ) {
-                            if(!(bidi.status.eor == QChar::DirR)) {
-                                // AN or EN
-                                appendRun( bidi );
-                                dir = QChar::DirR;
-                            }
-                            else
-                                bidi.eor = bidi.last;
+                        if(bidi.context->dir == QChar::DirL || bidi.status.lastStrong == QChar::DirL) {
+                            dir = QChar::DirR; // stuff before neutrals must have been R
                             appendRun( bidi );
                             dir = QChar::DirL;
+                            bidi.eor = bidi.current;
                             bidi.status.eor = QChar::DirL;
                         } else {
-                            if(bidi.status.eor == QChar::DirR) {
-                                appendRun( bidi );
-                                dir = QChar::DirL;
-                            } else {
-                                bidi.eor = bidi.current; bidi.status.eor = QChar::DirL; break;
-                            }
+                            bidi.eor = bidi.last;
+                            appendRun( bidi );
+                            dir = QChar::DirL;
+                            bidi.status.eor = QChar::DirL;
                         }
                     } else {
                         bidi.eor = bidi.current; bidi.status.eor = QChar::DirL;
@@ -911,16 +904,18 @@
                 case QChar::DirON:
                     if( !(bidi.status.eor == QChar::DirR) && !(bidi.status.eor == QChar::DirAL) ) {
                         //last stuff takes embedding dir
-                        if(bidi.context->dir == QChar::DirR || bidi.status.lastStrong == QChar::DirR) {
+                        if(bidi.context->dir == QChar::DirR || bidi.status.lastStrong == QChar::DirR
+                            || bidi.status.lastStrong == QChar::DirAL) {
+                            dir = QChar::DirL; // stuff before neutrals must have been L
                             appendRun( bidi );
                             dir = QChar::DirR;
                             bidi.eor = bidi.current;
-			    bidi.status.eor = QChar::DirR;
+                            bidi.status.eor = QChar::DirR;
                         } else {
                             bidi.eor = bidi.last;
                             appendRun( bidi );
                             dir = QChar::DirR;
-			    bidi.status.eor = QChar::DirR;
+                            bidi.status.eor = QChar::DirR;
                         }
                     } else {
                         bidi.eor = bidi.current; bidi.status.eor = QChar::DirR;
@@ -940,10 +935,7 @@
             if(!(bidi.status.lastStrong == QChar::DirAL)) {
                 // if last strong was AL change EN to AN
                 if(dir == QChar::DirON) {
-                    if(bidi.status.lastStrong == QChar::DirAL)
-                        dir = QChar::DirAN;
-                    else
-                        dir = QChar::DirL;
+                    dir = QChar::DirL;
                 }
                 switch(bidi.status.last)
                     {
@@ -1115,7 +1107,6 @@
                 break;
             case QChar::DirEN:
                 if ( bidi.status.last == QChar::DirL ) {
-                    bidi.status.last = QChar::DirL;
                     break;
                 }
                 // fall through


-- Mitz Pettel

[prev in list] [next in list] [prev in thread] [next in thread]