Bugzilla – Full Text Bug Listing
| Summary: | [New Widget] BidiLayout - Visual field supporting proper display and editing of Bidi data in visual Bidi layout | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Ira Fishbein <fira> |
| Component: | Nebula | Assignee: | Thomas Schindl <tom.schindl> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | fira, Lina.Kemmel, sadir, tom.schindl, tomerm |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=298392 | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
Ira Fishbein
I am copying here some sections from the description of Bug298392, which appears in 'see also' section. The description was written by Tomer Mahlin. Bidi text is stored differently on different platforms. For example: On Windows it is stored in logical Bidi layout While on mainframes it is stored in visual Bidi layout. On different platforms the mechanism used for displaying the data can be different. For example: On Windows, the text in the buffer is first transformed (using Unicode Bidi algorithm) and only then displayed on the screen. On Mainframe the text in the buffer is displayed on the screen as-is Each platform supports proper display of Bidi data in standard for this platform Bidi layout. In other words: Windows: supports proper display of Bidi data stored in logical Bidi layout (aka typing order or chronological order) Mainframe: supports proper display of Bidi data stored in visual Bidi layout Display of data on the platform when this data is stored in Bidi layout different from the Bidi layout supported by this platform results in incorrect display of Bidi data. One of the solution for this problem is providing means for display and editing of data natural to the platform from which it originates. In other words, for proper display and editing of Bidi data in visual Bidi layout (coming from mainframe) on Windows OS (which by default supports only proper display and editing of Bidi data in logical Bidi layout) we need to develop controls which support proper display and editing of visual data on Windows OS. One of the basic such controls supporting editing of visual data is called - visual field. Thus the expectation is to have SWT control / widget which: 1. Can be configured via API to be single / multi line 2. Can be interactively or via API configured to support various features of Visual input field (i.e. push mode) 3. Can be embedded and used as cell editor in DTP (and other plugins) 4. Can be used as standalone widget in Eclipse based application working with visual data. Created attachment 167081 [details]
zip file containing BidiLayout plugin
zip file containing BidiLayout plugin
Hi Ira - looks like a great addition to Nebula. Just one question, you are using OS-Class to get specific informations which makes your control only compile on with SWT-Win32. Do you know if this information is also available on other systems? Another question is whether you want to support the widget in future or only contribute it and we need to take care of it. I'll take a more close look at the code and come back to you until next week. Created attachment 167682 [details]
zip file containing BidiLayout plugin (without OS dependency)
Hi Tom,
I've removed OS dependency.
Regarding your second question - Yes, I am planning to support this widget in the future.
Ira
Hi Tom, Have you had a chance to look at the code? Please update me, Thank you, Ira I took a short look and there's one thing I don't really like with the current implementation. You are subclassing Canvas but IMHO this not needed at all so IMHO it would better to make your code not inherit from Canvas (you are also doing strange things in the constructor where you under certain cirtumstances add the styled-text-control on the parent and sometimes on the Canvas)
I'd suggest you refactor your code to look like this:
---------8<---------
public class BidiViewer {
public BidiViewer(Composite parent, int style) {
styledText = new StyledText(parent, style);
addBidiSegmentListener();
addListeners();
styledText.setMenu(createContextMenu());
}
}
---------8<---------
Hi Tom, Appreciate your prompt response. The general idea is to use this Widget in different Eclipse components. The first candidate for this is DTP. I want to make minimum changes in DTP code and BidiViewer is actually replacing there Text class. Thus I want my class to be 'Control' type. Another reason is that this way I inherit all Canvas methods and this also minimizes required changes in DTP code (no cast is required when Canvas methods are called). 'Strange things' in constructor also have the same explanation. This code will be used from different places and this logic is required. I can copy here code examples from DTP if I haven't explained myself clear enough. If you can suggest some other way that achieves similar results - please do. Thank you, Ira Hi Tom, Any update? Ira (In reply to comment #8) > Hi Tom, > Any update? > Ira Ira, I understand your problem with integrating this as a direct replacement but IMHO this wrapping has to happen outside the widget. As stated before being a subclass of Composite doesn't feel right for me. If you really really need to make it a subclass for Composite you need at least fix your constructor to make the inner styled text a child of the wrapping composite IN ANY CASE! Still I'd like you to think about others who'd like to use the widget. Is it really such a big overhead for you to make the "Control" something like a BidiViewer which is not inheriting from Composite but getting passed a Composite as the parent and for your purpose you create a Composite-Wrapper in your own code. This make the "Control" API much leaner and cleaner. Created attachment 172854 [details]
zip file containing BidiLayout plugin(with modified constructor)
Hi Tom,
I've changed constructor as you requested. New version is attached.
Regarding your suggestion 'to think about others who'd like to use this widget'. I am trying :). I've used this widget for DTP and it seems to me that the fact of being 'Control' actually leaves the code cleaner.
For example, in DefaultExternalTableDataWizardPage class :
getEditWidget().setLayoutData(gdtxtTextValue);
Thank you,
Ira
Tom, Just one more remark. I am not sure I explained it earlier. This widget will be used for as replacement of Text & StyledText controls. That is why it seems important for me to make it Control and to override methods from Text & StyledText classes. This will allow to keep caller's code cleaner, although it 'overloads' my widget's code. Ira Ira - CQ is filed as CQ 4304 - I'll now start the committer voting. One small note: Probably subclassing Composite instead of Canvas makes more sense. > One small note: Probably subclassing Composite instead of Canvas makes more
> sense.
We should not make this change before we brought in your code to CVS because the one with Canvas is sent for IP review
Created attachment 173499 [details]
zip file containing BidiLayout plugin(with modified constructor, subclassing Composite)
zip file containing BidiLayout plugin(with modified constructor, subclassing Composite)
Ira, the IP Team wants us to: --------8<-------- Hi Thomas: Please arrange for the contributor to make the following confirmations via the referenced bug. When that is in place, please let me know via this CQ. Confirmations: 1. Authored 100% of the content being contributed 2. Has the rights to donate the content to Eclipse 3. Contributes the content under the EPL Thanks, Sharon --------8<-------- Can you please answer those questions so that I can go ahead with the IP-Team. Tom (In reply to comment #15) Hi Tom, Yes, I confirm the following 3 statements. > Confirmations: > 1. Authored 100% of the content being contributed > 2. Has the rights to donate the content to Eclipse > 3. Contributes the content under the EPL Thank you, Ira Hi Tom, At this point, am I suppose to see my code in CVS? I couldn't find it... Thank you, Ira (In reply to comment #17) > Hi Tom, > At this point, am I suppose to see my code in CVS? I couldn't find it... > Thank you, > Ira You are the one responsible for checking it in :-) Once you've done it you can close this bug. The repo is found in: Host: dev.eclipse.org CVSROOT: /cvsroot/technology Module: org.eclipse.swt.nebula/org.eclipse.nebula.widgets.bidilayout The org.eclipse.nebula.widgets.bidilayout doesn't exists but the CVS-Share dialog helps you creating it. Done, Thank you! |