Bug 274567 - [Palette] BIDI3.5:Wrong shaping for digits in Customize Palette dialog and in Logic editor
Summary: [Palette] BIDI3.5:Wrong shaping for digits in Customize Palette dialog and in...
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy GEF (MVC) (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 339090
Blocks: 135242 307328
  Show dependency tree
 
Reported: 2009-04-30 14:35 EDT by Dina Sayed CLA
Modified: 2012-12-18 07:19 EST (History)
10 users (show)

See Also:


Attachments
Snap shots for the defects (263.44 KB, application/x-zip-compressed)
2009-04-30 14:35 EDT, Dina Sayed CLA
no flags Details
Proposed patch (3.66 KB, text/plain)
2011-03-31 11:40 EDT, Dina Sayed CLA
no flags Details
Proposed Patch (1.02 KB, patch)
2011-08-15 07:05 EDT, Ghada Selim CLA
no flags Details | Diff
Proposed patch for ICU shaping in solution2 (4.86 KB, patch)
2011-11-29 06:37 EST, Ghada Selim CLA
no flags Details | Diff
Snapshots (100.04 KB, application/x-rar)
2011-11-29 06:39 EST, Ghada Selim CLA
no flags Details
text flow patch (2.08 KB, patch)
2012-10-22 06:46 EDT, Yehia Abo Sedera CLA
no flags Details | Diff
bidi processor patch (1.82 KB, patch)
2012-10-22 06:46 EDT, Yehia Abo Sedera CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dina Sayed CLA 2009-04-30 14:35:45 EDT
Created attachment 133993 [details]
Snap shots for the defects

Build ID: I20090313-0100

Steps To Reproduce:
Platform : Windows vista

1-Open the eclipse
2-Go to File->New->Other->General->Project 
3-Add Arabic name mixed with numbers for the project name and then press finish

4-Go to File->New->Example->GEF->Logic diagram and hit Next. 
Be sure to select Logic Diagram  and not Logic under the GEF (Graphical Editing Framework) Plug-ins
5- give the new file an Arabic name.
6-Create a label in the logic editor that opens up (by dragging a label - under components - from the palette).  
7-Edit the label text in the logic editor and enter an Arabic text mixe with digits

Check Result 1

8-Right-click anywhere on the palette and select "Customize" from the pop-up menu.
9-In the dialog that pops up, select the tree item labeled "Components." You should be able to edit its name and description
 by typing Arabic text and numbers in the areas on the right-hand side of the tree
10-press ok to close the dialog 
11-View the changes you applied on the palette component name and hover the mouse to check the description

Check result 2




Expected Results


Result 1:Arabic text and numerals are displayed and shaped correctly in the label inside the logic editor
Result 2:Arabic text and numerals are displayed and shaped correctly



Actual Results:

Result 1:When editing in the text in the label the numerals are shaped correctly (check screen shot LabelInLogicEditor_underFocus_Result1.JPG) 
However the numerals are shaped wrongly it is shaped as Arabic European digits instead of Arabic indic digits after losing the focus  (check screen shot LogicEditor_Result1.JPG)

Result 2:When editing in the customize palette editor ,Arabic and numeral are displayed and shaped correctly(check screen shot CustomizeEditingdialog_Result2.JPG)
However after pressing ok and hover over the component to check the new description for the component , the numerals are shaped wrongly
it is shaped as Arabic European digits instead of Arabic indic digits(check screen shot CustomizeEdit_Result2.JPG)

More information:
Comment 1 Anthony Hunter CLA 2009-04-30 15:03:20 EDT
Resolution of BIDI defects are not in plan for Galileo. BIDI in text fields in
SWT applications are limited by issues in the base platform and there is no
plan to address these issues in Galileo.

Leaving open in case BIDI defects are looked at in a future release.
Comment 2 Dina Sayed CLA 2011-03-07 07:33:42 EST
Hi Anthony, 
I am interested in contributing to fix this bug, Would you direct me for what was reached so far from SWT limitation so that I may continue?
Comment 3 Dina Sayed CLA 2011-03-31 11:40:02 EDT
Created attachment 192295 [details]
Proposed patch

Part of this fix will depend on SWT 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=339090
Comment 4 Ghada Selim CLA 2011-08-15 07:05:15 EDT
Created attachment 201482 [details]
Proposed Patch
Comment 5 Ghada Selim CLA 2011-08-15 07:08:07 EDT
I believe that the call org.eclipse.draw2d.text.TextFlow.contributeBidi(BidiProcessor proc) is responsible for dividing the text into fragments and then each fragment is passed individually to be drawn, if a fragment contains numbers, then it is not passed within its context and therefore it will not be shaped correctly

Now I have a few questions
1- What is the purpose of BidiProcessor?
2- why it was added, what kind of bidi issues it solve and why it is needed in TextFlow?

As far as i see stopping its calls solve the problem and do not cause any issue
please check the proposed patch
Comment 6 Dina Sayed CLA 2011-10-11 16:33:40 EDT
(In reply to comment #4)
> Created attachment 201482 [details]
> Proposed Patch

Would anyone at least check this patch? fixing this bug will also fix a number of related bugs
Comment 7 Alexander Nyßen CLA 2011-10-11 17:06:58 EDT
(In reply to comment #6)
> (In reply to comment #4)
> > Created attachment 201482 [details] [details]
> > Proposed Patch
> 
> Would anyone at least check this patch? fixing this bug will also fix a number
> of related bugs

Well, I don't know much about Bidi (and I don't read or speak Arabic), but according to the Javadoc of BidiProcessor#process(), it is responsible of the following: 

* Processes the contributed text, determines the Bidi levels, and assign
* them to the FlowFigures that made thet contributions. This class is for
* INTERNAL use only. Shaping of visually contiguous Arabic characters that
* are split in different figures is also handled. This method will do
* nothing if the contributed text does not require Bidi evaluation. All
* contributions are discarded at the end of this method.

Thus, I doubt that simply removing the call is a valid patch.
Comment 8 Alexander Nyßen CLA 2011-10-11 17:10:13 EDT
(In reply to comment #3)
> Created attachment 192295 [details]
> Proposed patch
> 
> Part of this fix will depend on SWT 
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=339090

Concerning this one it seems that the underlying SWT changes have not yet been applied. What could we do beforehand?
Comment 9 Ghada Selim CLA 2011-11-29 06:36:10 EST
I can see that there are two proposed solutions for this bug:
1)Solution 1: Stopping the call to org.eclipse.draw2d.text.BidiProcessor in 
org.eclipse.birt.report.designer.internal.ui.editors.schematic.figures.TextFlow in case of Arabic locale.(refer to the patch in comment 4).

2)Solution 2: Shaping the digits using ICU to be in Arabic form in case of locale = Arabic and digit substitution = Context. (Please Check the attached patch which is based on the proposed fix in comment 3). This solution introduces a new method[isContextDigitSubistitution()] in "org.eclipse.swt.graphics.TextLayout" in SWT to check the digit substitution and then it is used by method [shapeArabicDigitsContextually(String text, int orient)]in org.eclipse.draw2d.text.BidiProcessor to perform the shaping. The method [isContextDigitSubistitution()] is added also to org.eclipse.swt.graphics.Resource which is the super class of org.eclipse.swt.graphics.TextLayout to avoid compilation errors in different platforms other than Windows which don't have this bug present(See the attached 
snapshots for Windows and Linux platforms after applying the fix).
So this solution requires changes in these classes:
    org.eclipse.draw2d.text.BidiProcessor
  org.eclipse.draw2d.text.TextFlow

  org.eclipse.swt.graphics.TextLayout
  org.eclipse.swt.graphics.Resource

Please advice.
Comment 10 Ghada Selim CLA 2011-11-29 06:37:15 EST
Created attachment 207645 [details]
Proposed patch for ICU shaping in solution2
Comment 11 Ghada Selim CLA 2011-11-29 06:39:04 EST
Created attachment 207646 [details]
Snapshots

Snapshots on Linux and Windows platforms for the fix in solution2
Comment 12 Yehia Abo Sedera CLA 2012-10-22 06:46:20 EDT
Created attachment 222628 [details]
text flow patch
Comment 13 Yehia Abo Sedera CLA 2012-10-22 06:46:54 EDT
Created attachment 222629 [details]
bidi processor patch
Comment 14 Yehia Abo Sedera CLA 2012-10-22 06:47:34 EDT
I have fixed the issue . kindly check attached patches (text flow & bidi processor )
Comment 15 Yehia Abo Sedera CLA 2012-12-18 07:19:49 EST
Hi All,
Kindly review the attached patch.
Thank you