Bug 93739 - RTL text is not alligned to the right
Summary: RTL text is not alligned to the right
Status: RESOLVED FIXED
Alias: None
Product: akregator
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
: 98188 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-11-22 20:16 UTC by Roie Kerstein
Modified: 2005-06-28 13:47 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
an example of broken article (1.75 KB, text/plain)
2005-02-02 18:14 UTC, Diego Iastrubni
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roie Kerstein 2004-11-22 20:16:44 UTC
Version:           1.0beta7 (using KDE KDE 3.3.1)
Installed from:    Gentoo Packages
Compiler:          GCC 3.4.3 
OS:                Linux

RTL (Hebrew) text is alligned to the left, instead of the right.
In lines with hebrew and english together, the order of the words is garbled, and the text is unreadable.

For example, see http://whatsup.org.il/backend.php
Comment 1 Frank Osterfeld 2004-11-22 22:12:39 UTC
CVS commit by osterfeld: 

fix alignment for RTL text
BUG: 93739


  M +6 -2      articleviewer.cpp   1.69
  M +0 -1      articleviewer.h   1.27


--- kdenonbeta/akregator/src/articleviewer.cpp  #1.68:1.69
@@ -215,11 +215,15 @@ QString ArticleViewer::formatArticle(Fee
     }
 
-    text += "<div id=\"body\">";
+    
 
     if (!article.description().isEmpty())
     {
+        text += QString("<div id=\"body\" dir=\"%1\">").arg(directionOf(article.description()) );
         text += "<span id=\"content\">"+article.description()+"</span>";
+        text += "</div>";
     }
 
+    text += "<div id=\"body\">";
+    
     if (article.commentsLink().isValid())
     {


Comment 2 Roie Kerstein 2004-11-29 12:49:48 UTC
Current fix is applied only when the first character of the message is of a RTL language.
If the first word of the message is in english, and all the rest is in RTL language it is still aligned to the left.
I would like to propose that each feed will include a definition whether it is LTR or RTL. This is a reasonable solution since one feed is always in one language, and there will not be mistakes in this way.
Comment 3 Frank Osterfeld 2004-11-29 12:59:34 UTC
> Current fix is applied only when the first character of the message is of a RTL language.

Confirmed. But how is text which contains both LTR and RTL aligned properly then? Are there any rules (maybe you've got a link)?

> I would like to propose that each feed will include a definition whether it is LTR or RTL. This is a reasonable solution since one feed is always in one language, and there will not be mistakes in this way. 

I guess you can't do that feed-wise in all cases, since a feed can contain items in different scripts and languages. What could be used as hint is the language property set in some feeds. 
Comment 4 Roie Kerstein 2004-11-29 13:09:50 UTC
> Confirmed. But how is text which contains both LTR and RTL aligned properly 
> then? Are there any rules (maybe you've got a link)? 

If you define 
text+="<div id=\"body\" dir=\"rtl">" 
instead of 
text+=QString("<div id=\"body\" dir=\"%1\">").arg(directionOf(article.description()) )
I believe that all articles will be alligned RTL unconditionally.
Now, if you define something like Feed::directionOf(), and determine a direction for the feed, you will be able to fix it without much effort. Just change it to:
QString("<div id=\"body\" dir=\"%1\">").arg(directionOf(article.getFeed()->directionOf() )

When doing this fix, please keep in mind that the same problem exists it the title of the article. Therefore, it makes sense to determine alignment for the feed, and not only for the article, or article text. With this approach, the title issue should be just as easily fixed.
Comment 5 Roie Kerstein 2004-11-29 13:25:31 UTC
By the way, KHTML knows how to align mixed RTL and LTR correctly, if you tell him the direction.
However, If you object to add the direction property to the feed, you can make it optional. I mean, for each feed let the user choose between "RTL, LTR, Aoutdetect", when Autodetect will be the way that it is now.
If you want to see an example of how mixed text is alligned properly when the first word is in english, please see http://whatsup.org.il/modules.php?op=modload&name=News&file=article&sid=3829&POSTNUKESID=14f67c748c86fc7f7373d61b66af34e5

The corresponding article is the one that begins with the words "Flash linux" in http://whatsup.org.il/backend.php. Also, if you click on "Complete Story", the whole webpage will be opened in a KHTML widget, and therefore it will be properly aligned.
Comment 6 Frank Osterfeld 2005-01-07 22:55:14 UTC
CVS commit by osterfeld: 

Ignore HTML tags when determining the RTL/LTR alignment of a string
CCBUG: 93739


  M +10 -5     articleviewer.cpp   1.82


--- kdepim/akregator/src/articleviewer.cpp  #1.81:1.82
@@ -40,4 +40,9 @@ static inline QString directionOf(const 
 }
 
+static inline QString stripTags(const QString& str)
+{
+    return QString(str).replace(QRegExp("<[^>]*>"), "");
+}
+
 int pointsToPixel(const QPaintDeviceMetrics &metrics, int pointSize)
 {
@@ -193,5 +198,5 @@ QString ArticleViewer::formatArticle(Fee
     if (!article.title().isEmpty())
     {
-        text += QString("<div id=\"headertitle\" dir=\"%1\">\n").arg(directionOf(article.title()));
+        text += QString("<div id=\"headertitle\" dir=\"%1\">\n").arg(directionOf(stripTags(article.title())));
         if (article.link().isValid())
             text += "<a id=\"titleanchor\" href=\""+article.link().url()+"\">";
@@ -220,5 +225,5 @@ QString ArticleViewer::formatArticle(Fee
     if (!article.description().isEmpty())
     {
-        text += QString("<div id=\"body\" dir=\"%1\">").arg(directionOf(article.description()) );
+        text += QString("<div id=\"body\" dir=\"%1\">").arg(directionOf(stripTags(article.description())) );
         text += "<span id=\"content\">"+article.description()+"</span>";
         text += "</div>";
@@ -308,5 +313,5 @@ void ArticleViewer::showSummary(FeedGrou
     QString text;
     text = QString("<div id=\"headerbox\" dir=\"%1\">\n").arg(QApplication::reverseLayout() ? "rtl" : "ltr");
-    text += QString("<div id=\"headertitle\" dir=\"%1\">%2").arg(directionOf(group->title())).arg(group->title());
+    text += QString("<div id=\"headertitle\" dir=\"%1\">%2").arg(directionOf(stripTags(group->title()))).arg(group->title());
     if(group->unread() == 0)
         text += i18n(" (no unread articles)");
@@ -327,5 +332,5 @@ void ArticleViewer::showSummary(Feed *f)
     text = QString("<div id=\"headerbox\" dir=\"%1\">\n").arg(QApplication::reverseLayout() ? "rtl" : "ltr");
 
-    text += QString("<div id=\"headertitle\" dir=\"%1\">").arg(directionOf(f->title()));
+    text += QString("<div id=\"headertitle\" dir=\"%1\">").arg(directionOf(stripTags(f->title())));
     text += f->title();
     if(f->unread() == 0)
@@ -348,5 +353,5 @@ void ArticleViewer::showSummary(Feed *f)
     if( !f->description().isEmpty() )
     {
-        text += QString("<div dir=\"%1\">").arg(directionOf(f->description()));
+        text += QString("<div dir=\"%1\">").arg(stripTags(directionOf(f->description())));
         text += i18n("<b>Description:</b> %1<br><br>").arg(f->description());
         text += "</div>\n"; // /description


Comment 7 Diego Iastrubni 2005-02-02 18:14:43 UTC
Created attachment 9395 [details]
an example of broken article

this rss feed contains 4 feeds:

1) english subject, and RTL content inside a p tag
2) same as 1, hebrew subject
3) same as 1, but with the letter "a" before the content
4) same as 2, but with the letter "a" before the content
Comment 8 Diego Iastrubni 2005-02-02 18:20:47 UTC
*** Bug 98188 has been marked as a duplicate of this bug. ***
Comment 9 Diego Iastrubni 2005-02-02 18:22:21 UTC
Try an open the rss I attached before.

feeds 1 and 2 look perfect with CVS HEAD.
feeds 3 and 4 do not. You have no control over it, and it's OK. If the RSS generators do not set the direction of the feeds, sometimes we will miss. That's OK and expected.

With akregator beta8, feeds 1 and 2 work wrong. 

IMHO, the bug is closed, and yes 98188 is a duplicate... my oops...

May I close this? I closed 98188 as duplicate of this.
Comment 10 Eckhart Wörner 2005-04-22 23:14:28 UTC
cuco reports in bug #104398 that this bug should be closed.
Comment 11 Diego Iastrubni 2005-04-23 16:45:24 UTC
Read my last post here. Poeple are complaining that it does not work, and I have explained why. You cannot fix this for everyone.

We are making hacks for displayind the text, but it does not always work. The best soluution is set the direcion from the server side, not guess it on the client.
Comment 12 Heinrich Wendel 2005-06-28 13:47:42 UTC
close as this is the best we can do