Issue 55927

Summary:

Edit engine draws BiDi text wrong

Product:

Calc

Reporter:

daniel.rentz

Component:

editing

Assignee:

stefan.baltzer

Status:

CLOSED FIXED

QA Contact:

issues@sc <issues>

Severity:

Trivial

Priority:

CC:

alan, hennerd, issues, kaplanlior, khirano, thomas.lange, xslf

Version:

680m132

Target Milestone:

---

Hardware:

All

OS:

All

Issue Type:

PATCH

Latest Confirmation in:

---

Developer Difficulty:

---

Attachments:

Description	Flags
tes document	none
patch for file svx/source/editeng/impedit.cxx	none
Screenshot before patch	none
Screenshot after patch	none

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.