Issue 55927 - Edit engine draws BiDi text wrong
Summary: Edit engine draws BiDi text wrong
Status: CLOSED FIXED
Alias: None
Product: Calc
Classification: Application
Component: editing (show other issues)
Version: 680m132
Hardware: All All
: P3 Trivial with 3 votes (vote)
Target Milestone: ---
Assignee: stefan.baltzer
QA Contact: issues@sc
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-13 15:50 UTC by daniel.rentz
Modified: 2013-08-07 15:15 UTC (History)
7 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
tes document (9.06 KB, application/vnd.oasis.opendocument.spreadsheet)
2005-10-13 15:53 UTC, daniel.rentz
no flags Details
patch for file svx/source/editeng/impedit.cxx (627 bytes, text/plain)
2008-03-30 09:52 UTC, hennerdrewes
no flags Details
Screenshot before patch (95.78 KB, image/png)
2008-06-11 08:33 UTC, hennerdrewes
no flags Details
Screenshot after patch (97.19 KB, image/png)
2008-06-11 08:35 UTC, hennerdrewes
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description daniel.rentz 2005-10-13 15:50:35 UTC
Consider a string containing latin characters (called L1, L2, ...), hebrew charcters 
(called H1, H2, ...) and punctuation characters (called P1, P2, ...). The string may 
contain the following characters, shown character by character from left to right:

L1 L2  H1 H2  P1 P2  L3 L4  P3 P4

In a left-to-right context, the string should be rendered as follows:

L1 L2  H2 H1  P1 P2  L3 L4  P3 P4  (hebrew RTL, punctuation and latin LTR)

but the edit engine shows the following:

L1 L2  H2 H1  P4 P3  L3 L4  P2 P1  (puncuation is RTL and at wrong position)

Order of characters changes wildly, if single punctuation or latin characters are 
formatted, e.g. with a text color.
Comment 1 daniel.rentz 2005-10-13 15:53:14 UTC
Created attachment 30404 [details]
tes document
Comment 2 frank.meies 2006-06-26 08:31:35 UTC
.
Comment 3 thomas.lange 2006-08-03 15:00:39 UTC
See issue 50657 for a similar problem.
Comment 4 Martin Hollmichel 2007-11-09 16:52:48 UTC
change target from 2.x to 3.x according to
http://wiki.services.openoffice.org/wiki/Target_3x
Comment 5 hennerdrewes 2008-03-30 09:50:59 UTC
The buggy bidi algorithm in the edit engine was discussed by fme in issue 37190.
A fix was proposed, but not implemented, because it did not seem to solve all
aspects of the problem.

I investigated the problem a little bit deeper, and I think, as far as the edit
engine is concerned, it is the right fix for issue 37190 and this issue.

I'll attach the patch as a reminder to fme. Please integrate this.

I'll post additional comments on issue 37190 on the cause of the remaining
problems over there.

Comment 6 hennerdrewes 2008-03-30 09:52:46 UTC
Created attachment 52366 [details]
patch for file svx/source/editeng/impedit.cxx
Comment 7 frank.meies 2008-03-31 08:36:13 UTC
fme->tl: Please take over this EditEngine issue.
Comment 8 hennerdrewes 2008-04-15 09:57:56 UTC
Anything happening with this patch?

Since this issue affects basic bidi functionality, it should be fixed as fast as
possible. Please consider retargeting. At least to 3.0, but preferably to 2.4.1.

Thanks a lot.

Comment 9 ooo 2008-04-28 12:14:19 UTC
@tl: Thomas, could you please take a look at this very short patch? Thanks, Eike.
Comment 10 thomas.lange 2008-04-30 08:57:33 UTC
Looks fine to me. But I need to test it a little which right now I have no time
for. Adding this issue to CWS tl53.
Comment 11 thomas.lange 2008-06-10 13:34:54 UTC
TL: Target back to OOo 3.0. We should always have the timer to review and test
such a tiny patch. ^_-
Comment 12 thomas.lange 2008-06-11 07:22:10 UTC
TL: This patch does not seem to change any at all.
I compared the results of the patch in DEV300m15 with the results of OOo 2.4
(SO8pp10) and they are identical. 


Also testing has no means to look in the internal order of the string
representation. Thus the 'is' and 'should' states need to be documented by
stating how the string will look like in LTR context and how is-state and/or
should-state are displayed in RTL context.
If I'm not mistaken in LTR it should be displayed as
  L1 L2  H4 H3  P5 P6  L7 L8  P9 P10
(The numerals indicating the keyboard input order)
And in RTL it should be displayed like this
  P10 P9 L7 L8 P6 P5 H4 H3 L1 l2
At least thats what I get when I do similar in writer.
Please correct me if I'm wrong.

Back to submitter.
Comment 13 hennerdrewes 2008-06-11 08:32:17 UTC
@tl: Are you sure you didn't miss anything?

I'll attach two screenshots: One before applying the patch and one after
applying the patch. I think the difference is obvious. 

Looking again at the attached patch, I realized that I made a mistake in naming
the patch file. I should be impedit3.cxx. Maybe the patch wasn't applied properly?
Comment 14 hennerdrewes 2008-06-11 08:33:26 UTC
Created attachment 54386 [details]
Screenshot before patch
Comment 15 hennerdrewes 2008-06-11 08:35:11 UTC
Created attachment 54388 [details]
Screenshot after patch
Comment 16 thomas.lange 2008-06-11 08:46:49 UTC
TL->hennerdrewes: Nope. I patched impedit3.cxx in DEV300m15 it was the line 4146.

But I see my mistake now. I compared the results of the patch and OOo 2.4 AFTER
setting those three paragraphs to RTL. And in that case there is no difference
between both versions.
Thus can you confirm that this patch does NOT fix anything when the paragraphs
are set to RTL? There are still problems with that case...
If you do so I will commit the patch right away.

Taking back ownership.
Comment 17 hennerdrewes 2008-06-11 09:46:43 UTC
This issue, according to its original description is solely on bidi text in LTR
environment, and thus the patch.

Just now I saw the problems when setting the paragraphs to RTL. You are right.
Hope to find time to have a look at that.
Comment 18 thomas.lange 2008-06-11 10:13:17 UTC
Fixed in CWS tl53. Thanks for the patch! :-)

Files changed:
- svx/source/editeng/impedit3.cxx
Comment 19 thomas.lange 2008-06-30 08:18:39 UTC
TL->SBA: Please verify. Thanks!
Comment 20 stefan.baltzer 2008-07-03 16:53:03 UTC
SBA: Verified in CWS tl53.
Comment 21 stefan.baltzer 2008-09-01 08:43:16 UTC
SBA: Re-verified in OOO300_m4. Closed.