Bug 161181 - RTL text shuffled when ellided in calendar cells
Summary: RTL text shuffled when ellided in calendar cells
Status: RESOLVED FIXED
Alias: None
Product: korganizer
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords: triaged
Depends on:
Blocks:
 
Reported: 2008-04-23 02:55 UTC by Shai
Modified: 2008-10-13 20:26 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
The cell when it is wide enough for the todo title (109.34 KB, image/png)
2008-04-23 02:57 UTC, Shai
Details
The cell when it is not wide enough for the todo title (108.71 KB, image/png)
2008-04-23 03:00 UTC, Shai
Details
Full vs. elided text in KDE4 Korganizer week view (19.48 KB, image/png)
2008-07-13 23:05 UTC, Shai
Details
The same picture, with marks showing which letter went where (21.91 KB, image/png)
2008-07-13 23:07 UTC, Shai
Details
Eliding always to the left still doesn't quite work (7.67 KB, image/png)
2008-07-14 23:41 UTC, Shai
Details
partial fix for month view (705 bytes, patch)
2008-09-26 17:59 UTC, Shai
Details
Fix KWordwrap, which mostly fixes week view (1.63 KB, patch)
2008-10-13 02:10 UTC, Shai
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shai 2008-04-23 02:55:36 UTC
Version:            (using KDE 3.5.9)
Installed from:    Debian testing/unstable Packages
OS:                Linux

In the calendar month view, appointments and todos have their title elided when it doesn't fit within the cell. The method of eliding is by making the last characters appear dimmed.

However, the dimmed characters are always shown at the right side of the cell. When the title is RTL (Hebrew in my case) this is inappropriate -- the characters are incorrectly reordered. I will soon attach the screen-caps showing the problem exactly, but I first translate it here to English to clarify:

Suppose my title were "Abcde Fghij". Now, I make the cell only large enough for 10 characters. The expected behavior is that I should see "Abcde F(ghi)", where the characters in parentheses are increasingly dimmed (g a little, h more, i most). Instead, in Hebrew, I see the equivalent of "(ihg)Abcde F", where "i" is the most dimmed.
Comment 1 Shai 2008-04-23 02:57:25 UTC
Created attachment 24483 [details]
The cell when it is wide enough for the todo title
Comment 2 Shai 2008-04-23 03:00:06 UTC
Created attachment 24484 [details]
The cell when it is not wide enough for the todo title

In the couple of screen caps you can see (if you look carefully at the Hebrew
characters) how the title becomes distorted when it is ellided -- compare it to
the english example given in the bug description.
Comment 3 Allen Winter 2008-07-13 00:47:25 UTC
SVN commit 831596 by winterz:

Fix "RTL text shuffled when ellided in calendar cells"
I hope.  

Shai and Dotan, please test.

BUG: 161181
CCMAIL: kde-2@dotancohen.com


 M  +11 -2     monthgraphicsitems.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=831596
Comment 4 Shai 2008-07-13 23:03:41 UTC
Hi Allen,

Thanks for trying, but no, this doesn't quite fix it. At least as far as I see, the bug has moved from the month view to the week view. 

The month view now uses eliding with an ellipsis, not dimming of late characters; in this mode, there's only a question of positioning the ellipsis w.r.t what's left of the text. Currently, in a LTR locale the ellipsis is before the text, but it is the beginning of the text that is cut out, so it looks ok (it is a little funny that it's the beginning that's removed, but we've seen much worse :-)). When there's a RTL (Hebrew) cell but the locale is LTR, the ellipsis now appears on the wrong side of the text (it probably has always appeared this way). Between the beginning of text with ellipsis on its wrong side, and the end of text with ellipsis on its right side, I'm not sure which I prefer; I think they're both non-comfortably acceptable, at least to an experienced computer user.

However, the problem, exactly as I described it, now lives in the Week view, where eliding is done by dimming the last characters. Make a longish title, and play with the event duration to make it elide; if the title is LTR, you can see the weird dance of letters as exemplified by the attachments I am now adding -- where the titles of the event and the todo are actually the same, but the event is elided.

My conclusion from the above is that the problem is in the QPainter, not in the use of elidedText. If I find the time, I'll look more deeply into it.

Either way, the new KOrganizer (like the rest of KDE4) is a lot prettier than the old one.

Thanks again,
Shai.
Comment 5 Shai 2008-07-13 23:05:53 UTC
Created attachment 26091 [details]
Full vs. elided text in KDE4 Korganizer week view
Comment 6 Shai 2008-07-13 23:07:07 UTC
Created attachment 26092 [details]
The same picture, with marks showing which letter went where
Comment 7 Shai 2008-07-13 23:08:43 UTC
(sorry, forgot to reopen with earlier post)
Comment 8 Shai 2008-07-13 23:18:27 UTC
... and again sorry for the spam, but I've just noticed what a mess I've made above between LTR and RTL. So here is the above again, hopefully corrected:

The month view now uses eliding with an ellipsis, not dimming of late characters; in this mode, there's only a question of positioning the ellipsis w.r.t what's left of the text. Currently, in a *RTL* locale the ellipsis is before the text, but it is the beginning of the text that is cut out, so it looks ok (it is a little funny that it's the beginning that's removed, but we've seen much worse :-)). When there's a RTL (Hebrew) cell but the locale is LTR, the ellipsis now appears on the wrong side of the text (it probably has always appeared this way). Between the beginning of text with ellipsis on its wrong side, and the end of text with ellipsis on its right side, I'm not sure which I prefer; I think they're both non-comfortably acceptable, at least to an experienced computer user. 

However, the problem, exactly as I described it, now lives in the Week view, where eliding is done by dimming the last characters. Make a longish title, and play with the event duration to make it elide; if the title is *RTL*, you can see the weird dance of letters as exemplified by the attachments I am now adding -- where the titles of the event and the todo are actually the same, but the event is elided. 

My conclusion from *examining the code changes* is that the problem is in the QPainter, not in the use of elidedText.

I hope this is now right and clear,
Shai.
Comment 9 Allen Winter 2008-07-14 01:29:02 UTC
I really want to fix this.
We have a couple problems: 
1) be consistent between month and week views (either dimming or ellipsis)
2) make sure the dimming (or ellipsis) is in the correct place

It seems that KWordWrap class (which does the dimming) does not properly support RtoL, probably due to QPainter.

So let's try to use ellipsis in both views.. at least for now.

If we can get the monthview correct, then I can make the similar fix in week view.

Please try this: in korganizer/views/monthgraphicsitem.cpp,
change the Qt::ElideLeft in both places to QT::ElideRight.
Let me know if that works.
Comment 10 Shai 2008-07-14 23:41:27 UTC
Created attachment 26132 [details]
Eliding always to the left still doesn't quite work

No, not quite. The problem is that the presentation relies on the global
ReverseLayout property, instead of the direction of the text in the cell. This
means that in a LTR (English) layout, RTL (Hebrew) text will come out wrong,
and vice versa; it's not an issue of the elide mode, but of presentation of
text.

The attached image was taken in RTL (Hebrew), to make it easier for non-Hebrew
users to see what's wrong. The full (English) event title is shown on the
right. The Hebrew event's eliding here is perfect. If I use a LTR layout, then
it's the other way around.
Comment 11 Shai 2008-07-14 23:44:08 UTC
Just to be perfectly clear: The eliding of the English title is elideLeft. If I used elideRight, it would show something like "...Event".
Comment 12 Allen Winter 2008-07-15 15:43:37 UTC
Shai,

What do you recommend for the 4.1 release?
Leave it at the current state? Or put back the changes I made for RTL?

There seem to be Qt problems underlying that won't be fixed before 4.1 is tagged in 1 week.
Comment 13 Shai 2008-07-16 22:43:27 UTC
Either way is fine in terms of UI; but I guess it is better to remove the elideRight references (and the KGlobals::reverseLayout references), in terms of hints for future people looking at this code.
Comment 14 Shai 2008-07-16 22:46:47 UTC
PS: But if you can get the Week view to elide by ellipsis, that will be an improvement.
Comment 15 Allen Winter 2008-07-19 17:31:47 UTC
SVN commit 834757 by winterz:

revert SVN commit 831596 by winterz:

Fix "RTL text shuffled when ellided in calendar cells"
I hope.  

==> Shai says it doesn't help to do this.  The real problem is in QPainter
CCBUGS: 161181


 M  +2 -11     monthgraphicsitems.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=834757
Comment 16 Shai 2008-09-26 17:59:23 UTC
Created attachment 27581 [details]
partial fix for month view

The attached 2-line patch fixes the worst effects of the bug for the month view; it causes the ellipsis to be printed on the correct side of the text, whether the text is Hebrew or English.

The Week view still does things differently (I don't see dimming anymore, just cropping of characters, but the cropping is still done as described in the bug, so that characters are shuffled around). I suppose one can put a similar fix there.

Once we get over the really-bad, we  can start worrying about when icons should appear on the right side of the cell instead of the left.
Comment 17 Allen Winter 2008-09-28 02:23:42 UTC
Thanks Shai.  I committed the 2-line patch.
Keep them coming!
Comment 18 Michael Leupold 2008-09-28 12:23:35 UTC
What's the status of this bug? Was it finally resolved back then?
Comment 19 Allen Winter 2008-09-28 14:16:13 UTC
Technically speaking this bug has been fixed.
However, as Shai mentions, there are other RTL issues in the cells (like icon placement)

So, let's leave the decision to Shai.  We can close and open other bugs, or just leave this one open.
Comment 20 Shai 2008-10-01 09:57:16 UTC
(In reply to comment #19)
> Technically speaking this bug has been fixed.
> However, as Shai mentions, there are other RTL issues in the cells (like icon
> placement)
> 
> So, let's leave the decision to Shai.  We can close and open other bugs, or
> just leave this one open.
> 

I think the icon placement should be dealt with in its own bug -- that really is a minor issue. However, I'd like to keep this one open until the week view situation is improved.
Comment 21 Shai 2008-10-13 02:10:45 UTC
Created attachment 27839 [details]
Fix KWordwrap, which mostly fixes week view

The attached patch fixes (the worst of) the problems in week view. It makes the dimmed characters paint in the correct place, and stops character shuffling. This leaves minor display problems, like alignment and icon placement, which are of much lower priority.

Notice that the patch is not to kdepim, but to kdelibs; it should solve similar problems all around KDE, though I personally haven't run into them.
Comment 22 Diego Iastrubni 2008-10-13 13:01:10 UTC
SVN commit 870851 by iastrubni:

This fixes (the worst of) the problems in week view in KOrganizer (and probably other places). 

It makes the dimmed characters paint in the correct place, and stops character shuffling. This 
leaves minor display problems, like alignment and icon placement, which are of much lower priority.

Patch by Shai berger.

BUG: 161181
CCMAIL: Shai Berger <shai@platonix.com>



 M  +24 -9     kwordwrap.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=870851
Comment 23 Diego Iastrubni 2008-10-13 20:26:35 UTC
SVN commit 871006 by iastrubni:

This fixes (the worst of) the problems in week view in KOrganizer (and probably other places). 

It makes the dimmed characters paint in the correct place, and stops character shuffling. 
This leaves minor display problems, like alignment and icon placement, which are of much lower 
priority. 

Patch by Shai berger. Backport for 4.1.

BUG: 161181 
CCMAIL: Shai Berger <shai@platonix.com> 




 M  +24 -9     kwordwrap.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=871006