Bug 87779 - [BIDI] Need to enable image lookup to be BIDI aware
Summary: [BIDI] Need to enable image lookup to be BIDI aware
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: 3.1 M7   Edit
Assignee: Tod Creasey CLA
QA Contact:
URL:
Whiteboard:
Keywords: nl
Depends on: 87802 87826
Blocks:
  Show dependency tree
 
Reported: 2005-03-11 10:42 EST by Tod Creasey CLA
Modified: 2005-05-11 16:53 EDT (History)
9 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tod Creasey CLA 2005-03-11 10:42:48 EST 
The following plug-ins will need to have thier image lookup nl-enabled for BIDI
support.

Right now we look it up in the form

 URL url = BundleUtility.find(PlatformUI.PLUGIN_ID, path);

it should be 

 URL url = BundleUtility.find("$nl$/" + PlatformUI.PLUGIN_ID, path);

so that OSGI looks up based on locale as well. I am not sure if this is has any
serious performance implications - if not then we should do it for all of our
image descriptors. If so then it should do it for only those we know we want to
override. Jeff please let me know.

org.eclipse.ant.ui_3.1.0
org.eclipse.compare_3.1.0
org.eclipse.debug.ui_3.1.0
org.eclipse.help.webapp_3.0.0
org.eclipse.jdt.debug.ui_3.1.0
org.eclipse.jdt.junit_3.1.0
org.eclipse.jdt.ui_3.1.0
org.eclipse.pde.runtime_3.1.0
org.eclipse.pde.ui_3.1.0
org.eclipse.platform_3.1.0
org.eclipse.platform.doc.user_3.1.0
org.eclipse.search_3.1.0
org.eclipse.team.cvs.ui_3.1.0
org.eclipse.team.ui_3.1.0
org.eclipse.ui_3.1.0
org.eclipse.ui.cheatsheets_3.0.0
org.eclipse.ui.editors_3.1.0
org.eclipse.ui.externaltools_3.1.0
org.eclipse.ui.ide_3.1.0
org.eclipse.ui.intro_3.1.0
org.eclipse.update.ui_3.0.0
Comment 1 Tod Creasey CLA 2005-03-11 10:43:34 EST 
Note - I will log seperate PRs to the other components when we are satisfied
with the workbenches answer.
Comment 2 Tod Creasey CLA 2005-03-11 10:53:51 EST 
Should be more like:

 public final static String ICONS_PATH = "$nl$/icons/full/";//$NON-NLS-1$

URL url = BundleUtility.find(PlatformUI.PLUGIN_ID, ICONS_PATH + "etool16/");
Comment 3 Tod Creasey CLA 2005-03-11 11:49:51 EST 
I did some rough testing of how adding $nl$ to the beginning of our paths
affects startup.

With nl enablement WorkbenchImages (92 images) takes around 60-70 ms.
With nl enablement but no fragments it takes around 110ms.
With nl enablement and fragments for ui and ide and no matching directories it
takes around 200ms.
With nl enablement and fragments for ui and ide using Emads overrides it takes
250-270 ms.

As the performance hit is quite small for workbenches with no fragments I think
we can consider appending $nl$ to all of our icon path lookups with performance
hits only on platforms lat have them.
Comment 4 Tod Creasey CLA 2005-03-11 13:42:16 EST 
We need to be sure than Bug 87802 will not block this
Comment 5 Tod Creasey CLA 2005-03-15 08:36:14 EST 
I have done metrics a few more times and the nl directory seems to make the
average startup time around 145 ms - the structure matters little (i.e. iw and
iw_IR are about the same) so I don't think the Platform lookup is hurting us
singnificantly.

With no  fragments it is about 110 ms. I'll do a full metric when the workbench
is  enabled.
Comment 6 Tod Creasey CLA 2005-04-28 11:19:04 EDT 
This was fixed earlier in the M7 stream. We have implemented the suggested method.
Comment 7 Tod Creasey CLA 2005-04-28 11:19:21 EDT 
Fixed in build 20050428
Comment 8 Tod Creasey CLA 2005-04-28 11:20:02 EDT 
And it should read BundleUtility.find(PlatformUI.PLUGIN_ID, "$nl$/" +  path);
Comment 9 Tod Creasey CLA 2005-05-11 16:53:58 EDT 
Verified in 20050511 on Tasks view