Bug 378813 - bidi2012 - Incorrect display of Java Project location containing Hebrew characters
Summary: bidi2012 - Incorrect display of Java Project location containing Hebrew chara...
Status: CLOSED DUPLICATE of bug 273728
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.8   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-08 07:04 EDT by Helena Halperin CLA
Modified: 2012-06-18 10:06 EDT (History)
8 users (show)

See Also:
markus.kell.r: review-


Attachments
typed the Hebrew directory (25.59 KB, image/jpeg)
2012-05-08 07:06 EDT, Helena Halperin CLA
no flags Details
Typed Hebrew directory and subdirectory (34.58 KB, image/jpeg)
2012-05-08 07:07 EDT, Helena Halperin CLA
no flags Details
Browse to Hebrew subdirectory (36.33 KB, image/jpeg)
2012-05-08 07:08 EDT, Helena Halperin CLA
no flags Details
Patch for eclipse.jdt.ui plugin (3.80 KB, patch)
2012-06-04 02:49 EDT, Ira Fishbein CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helena Halperin CLA 2012-05-08 07:04:38 EDT
Build Identifier: version 3.8.0 build I20120314-1800

Location path in "New Java Project" dialog is displayed incorrect when contains Hebrew characters (dynamic complex expression)

Reproducible: Always

Steps to Reproduce:
This is Hebrew specific problem
1. Create a directory and subdirectory with Hebrew names
2. Open Eclipse
3. Navigate to File->New->Java Project
4. In the "location" field, type the name of Hebrew directory, then Hebrew subdirectory. Or browse to Hebrew subdirectory. 
Inspect the location path.

result: the path display is incorrect, the directory and subdirectory are switched.
Comment 1 Helena Halperin CLA 2012-05-08 07:06:32 EDT
Created attachment 215242 [details]
typed the Hebrew directory
Comment 2 Helena Halperin CLA 2012-05-08 07:07:13 EDT
Created attachment 215243 [details]
Typed Hebrew directory and subdirectory
Comment 3 Helena Halperin CLA 2012-05-08 07:08:01 EDT
Created attachment 215244 [details]
Browse to Hebrew subdirectory
Comment 4 Grant Gayed CLA 2012-05-17 11:35:04 EDT
Presumably the problem is that the Text is applying some RTL rules that it should not since its content represents a file system location.
Comment 5 Grant Gayed CLA 2012-05-17 14:41:57 EDT
I don't currently expect this to be addressed for 3.8, so setting target milestone to 4.2.1, which is the next planned maintenance release.
Comment 6 Grant Gayed CLA 2012-05-17 15:37:12 EDT
Lina is this a case where a BidiSegmentListener added at the UI level could help?
Comment 7 Lina Kemmel CLA 2012-05-22 08:05:23 EDT
(In reply to comment #6)
> Lina is this a case where a BidiSegmentListener added at the UI level could
> help?

Grant, BidiSegmentListener works for StyledText only. I think here we should use org.eclipse.swt.events.SegmentListener.
We will try to create proposed patch for this and similar bugs if you are OK with it.
Comment 8 Grant Gayed CLA 2012-05-22 10:23:39 EDT
(In reply to comment #7)
> We will try to create proposed patch for this and similar bugs if you are OK
> with it.

Yes, absolutely.
Comment 9 Ira Fishbein CLA 2012-06-04 02:49:45 EDT
Created attachment 216748 [details]
Patch for eclipse.jdt.ui plugin

Please find attached patch with solution for reported problem.
Ira
Comment 10 Grant Gayed CLA 2012-06-14 16:05:20 EDT
Changes are at JDT's UI level, so moving there to review and commit.  The approach is correct in that UI should do the work in the SegmentListener since it knows that the string represents a path (note that we have not reviewed the changes in any detail).
Comment 11 Markus Keller CLA 2012-06-14 16:53:44 EDT
Does all this work for non-Java projects?
- "New Project" dialog
- project properties > Resource: Path/Location fields

JDT/UI will not start working on bidi problem unless these issues have been successfully fixed in the platform in a clean way.

The patch is clearly a no-go. JDT will never release any locale-specific code. The only acceptable API is a single method that takes a Text widget.
Comment 12 Tomer Mahlin CLA 2012-06-14 17:14:40 EDT
Hi Markus,

I would appreciate if you can clarify the terms / conditions under which the patch will be accepted by JDT. I find the phrase " ... JDT/UI will not start working on bidi problem unless these issues have been successfully fixed in the platform in a clean way ..." very ambiguous. 
  The problem was solved on the platform in a clean way via bug 183164 (providing parsers) and bug 230854 (providing support for SegmentListener in Text widget).

A fix similar to the one suggested for JDT will of course work for any other non Java project. However separate fixes will need to be made for different contexts since fix works on Text widget instance level. Not all Text widgets are used for path specification and thus fixes should be very isolated to very specific contexts.

As far as I understand from you, the preconditions for accepting our fix are:
1. Make sure there are no locale specific code (remove locale dependency in the suggested fix)
2. Make sure similar issue is fixed in the following contexts:
- "New Project" dialog
- project properties > Resource: Path/Location fields

Please confirm.
Thanks in advance.
Comment 13 Markus Keller CLA 2012-06-14 17:31:34 EDT
Sorry, when I said "platform", I meant "Platform UI" and "Platform IDE". I suppose the non-Java project dialogs have the same problems, and those problems need to be solved first.

While fixing the problem in Platform UI, you will have to come up with an API that just takes a Text widget and then applies all the necessary infrastructure to correctly handle bidi paths in that Text.

Once this is ready, JDT UI will just have to add a call to that new API method to fix the problem for a path edit field in JDT UI. Code duplication needs to be avoided at all cost, so nothing of this implementation will live in JDT UI (since a "filesystem path" is not JDT-specific).
Comment 14 Dani Megert CLA 2012-06-15 02:21:10 EDT

*** This bug has been marked as a duplicate of bug 196085 ***
Comment 15 Markus Keller CLA 2012-06-15 05:05:56 EDT
Not a dup. Java package != filesystem path.
Comment 16 Markus Keller CLA 2012-06-15 05:15:13 EDT
Another point to consider: The check whether bidi support is needed must be somewhere in the bidi bundle, and its result must be cached so that the bidi enhancements have zero overhead on LTR platforms.
See org.eclipse.osgi.util.TextProcessor.IS_PROCESSING_NEEDED.
Comment 17 Dani Megert CLA 2012-06-15 06:49:51 EDT
(In reply to comment #15)
> Not a dup. Java package != filesystem path.

Yes, I pasted the wrong number.

*** This bug has been marked as a duplicate of bug 273728 ***
Comment 18 Ira Fishbein CLA 2012-06-18 07:54:08 EDT
Hi Markus,
In order to handle similar situation for non-Java projects, bug 381487 was opened (and patch was provided).
Regarding the API you suggested - where do you think this should be implemented? I thought to add this to org.eclipse.equinox.bidi plugin (this plugin handles Bidi-specific issues), but it currently has no dependency on SWT and I am not sure that is good idea to add such dependency.
Please advise.
Ira
Comment 19 Markus Keller CLA 2012-06-18 10:06:08 EDT
I followed up in bug 381487. Let's continue the discussions there.