Patch to fix bug 134036

Shai Berger <[email protected]> Sat, 26 Apr 2008 01:34:21 +0300

Newsgroups	gmane.comp.kde.internationalization.hebrew,gmane.comp.kde.devel.pim
Message-ID	<[email protected]>

Hi Pimsters,

A couple of months ago I came to this list to mention some BiDi problems in 
KMail trunk, and some solutions to them  that I had. I've been following the 
list since then, and noted the advice that patches posted only in bugzilla 
tend to stay there, but I couldn't find the time to revalidate my patch so I 
could post it here.

But now, I have found the time, so I'm attaching an updated  version of the 
patch against trunk. The horrible regressions are still there, as far as I 
can tell (starting a paragraph in Hebrew does not make it RTL in the editor, 
as it does in 3.5 -- and this is probably a Qt issue), but this patch 
improves the handling of mixed paragraphs. The full details are in 
http://bugs.kde.org/show_bug.cgi?id=134036, but at a sound-byte level, if you 
read this message in kmail, then the next line should appear right-justified 
שלום because it starts with a Hebrew word, and if it had some more hebrew in 
it, its words would be thrown out of order too; the patch improves this by 
guessing when a new line is really a new paragraph (issue can't be solved  
completely because paragraphs are not well-defined in plaintext mail).

Thanks for your good work, and have fun,

	Shai.

------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/kde-il/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/kde-il/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:[email protected] 
    mailto:[email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/

kmail-bidi.patch (text/x-diff, 3.8 KB)

Index: kmail/objecttreeparser.h
===================================================================
--- kmail/objecttreeparser.h	(revision 801157)
+++ kmail/objecttreeparser.h	(working copy)
@@ -260,6 +260,11 @@
     QString quotedHTML(const QString& pos, bool decorate);
 
     const QTextCodec * codecFor( partNode * node ) const;
+    /** Check if position @p pos in string @p s seems to
+        separate two paragraphs (important for correct BiDi
+        behavior, but is heuristic because paragraphs are
+        not well-defined */
+    bool looksLikeParaBreak(const QString& s, unsigned int pos) const;
 
 #ifdef MARCS_DEBUG
     void dumpToFile( const char * filename, const char * dataStart, size_t dataLen );
Index: kmail/objecttreeparser.cpp
===================================================================
--- kmail/objecttreeparser.cpp	(revision 801157)
+++ kmail/objecttreeparser.cpp	(working copy)
@@ -2711,6 +2711,8 @@
 
   unsigned int pos, beg;
   const unsigned int length = s.length();
+  bool paraIsRTL;
+  bool startNewPara = true;
 
   // skip leading empty lines
   for ( pos = 0; pos < length && s[pos] <= ' '; pos++ )
@@ -2829,12 +2831,19 @@
       // ignore ^M DOS linebreaks
       if( !line.replace('\015', "").isEmpty() )
       {
-         htmlStr +=QString( "<div dir=\"%1\">" ).arg( line.isRightToLeft() ? "rtl":"ltr" );
+         if (startNewPara)
+           paraIsRTL = line.isRightToLeft();
+         htmlStr +=QString( "<div dir=\"%1\">" ).arg( paraIsRTL ? "rtl":"ltr" );
          htmlStr += LinkLocator::convertToHtml( line, convertFlags );
          htmlStr += QString( "</div>" );
+         startNewPara = looksLikeParaBreak(s,pos);       
       }
       else
+      {              
         htmlStr += "<br>";
+        // after an empty line, always start a new paragraph
+        startNewPara = true;              
+      }       
     }
   } /* while() */
 
@@ -2859,6 +2868,57 @@
     return node->msgPart().codec();
   }
 
+  bool ObjectTreeParser::looksLikeParaBreak(const QString& s, unsigned int pos) const
+  {
+    const unsigned int WRAP_COL = 78;
+    unsigned int length = s.length();
+    if (pos>=length-1 || pos==0)
+      return false;
+    unsigned prevStart = s.lastIndexOf('\n', pos-1, Qt::CaseInsensitive);
+    prevStart++; // First char of prev line -- works also for first line
+    unsigned lastLineLength = pos - prevStart;
+    if (lastLineLength>WRAP_COL)
+      return true;
+    
+    /* search next occurrence of '\n' */
+    unsigned int end = s.indexOf('\n', pos+1, Qt::CaseInsensitive);
+    if (end == (unsigned int)(-1))
+      end = length;
+    QString nextLine = s.mid(pos,end-pos);
+    length = nextLine.length();
+    /* search for first word in next line */
+    unsigned int wordStart;
+    bool found = false;
+    for (wordStart=0; !found && wordStart<length; wordStart++) {
+      switch (nextLine[wordStart].toLatin1()) {
+        case '>':
+        case '|':
+        case ' ':  // spaces, tabs and quote markers don't count
+        case '\t':
+        case '\r':
+          break;
+        default:  
+          found = true;
+          break;
+      }
+    } /* for() */
+    if (!found)
+      return true; // next line is actually empty, it seems
+    
+    //Find end of first word.
+    //Note: flowText (in kmmessage.cpp) separates words for wrap by
+    //spaces only. This should be consistent, which calls for some
+    //refactoring.
+    unsigned int wordEnd = nextLine.indexOf(' ', wordStart, Qt::CaseInsensitive);
+    int wordLength;
+    if (wordEnd==(unsigned int)(-1))
+      wordLength = length;
+    else
+      wordLength = wordEnd-wordStart;
+      
+    return lastLineLength+wordLength+1 < WRAP_COL;  
+  }
+  
 #ifdef MARCS_DEBUG
   void ObjectTreeParser::dumpToFile( const char * filename, const char * start,
                                      size_t len ) {