Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 311589

Summary:

[Help] CriterionValueDefinition has bad copy constructor

Product:

[Eclipse Project] Platform

Reporter:

Chris Goldthorpe <cgold>

Component:

User Assistance

Assignee:

Chris Goldthorpe <cgold>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

zhhaohh

Version:

3.6

Flags:

zhhaohh: review+

Target Milestone:

3.6 RC1

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Test case	none
Patch containing a fix for the problem and new tests	none

Description Chris Goldthorpe

2010-05-04 13:26:31 EDT

I20100429-1549

CriterionValueDefinition contains a "copy constructor" which is used is used to create a CriteriaValueDefinition from an ICriteriaValueDefinition. 

The signature is:

public CriterionValueDefinition(ICriterionDefinition src)

which is incorrect since the parameter should be of type ICriteriaValueDefinition. This cause the UAElementFactory to return null when creating an object.

I will attach a test case I used to detect the problem.

Comment 1 Chris Goldthorpe

2010-05-04 13:40:09 EDT

Created attachment 166989 [details]
Test case

To reproduce:

1. Apply this patch to org.eclipse.ua.tests
2. Checkout org.eclipse.sdk and add the following lines to plugin_customization.ini

# Filtering by criteria
# Set true to enable criteria filtering, otherwise set false
org.eclipse.help/enableCriteria=true

# List all the supported criteria names, separated by comma 
org.eclipse.help/supportedCriteria=containsLetter

3. Launch the Eclipse SDK
4. Help/Help contents from the SDK that was just launched
5. Edit a scope - the title "Criteria" shows in the scope dialog but no criteria are shown, in the log there is an exception:

java.lang.reflect.InvocationTargetException
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:80)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java(Compiled Code))
at java.lang.reflect.Constructor.newInstance(Constructor.java(Compiled Code))
at org.eclipse.help.internal.UAElementFactory.newElement(UAElementFactory.java:122)
at org.eclipse.help.internal.criteria.CriteriaDefinitionManager.readCriteriaDefinitionContributions(CriteriaDefinitionManager.java:104)
at org.eclipse.help.internal.criteria.CriteriaDefinitionManager.getCriteriaDefinition(CriteriaDefinitionManager.java:49)
at org.eclipse.help.internal.criteria.CriteriaDefinitionManager.getCriterionName(CriteriaDefinitionManager.java:161)
at org.eclipse.help.internal.criteria.CriteriaManager.getCriterionDisplayName(CriteriaManager.java:99)
at org.eclipse.help.internal.workingset.WorkingSetManager.getCriterionDisplayName(WorkingSetManager.java:509)
at org.eclipse.help.internal.webapp.servlet.WebappWorkingSetManager.getCriterionDisplayName(WebappWorkingSetManager.java:140)
at org.eclipse.help.internal.webapp.data.WorkingSetData.getCriterionDisplayName(WorkingSetData.java:212)
at org.apache.jsp.advanced.workingSet_jsp._jspService(workingSet_jsp.java:796)

Comment 2 Chris Goldthorpe

2010-05-04 14:47:31 EDT

Created attachment 167015 [details]
Patch containing a fix for the problem and new tests

This patch contains the fix for this bug and also adds some new Junit tests as well as adding some criterion definitions to org.eclipse.ua.tests which can be used in non-automated testing.

Comment 3 Chris Goldthorpe

2010-05-04 14:50:31 EDT

Vivian, please review "Patch containing a fix for the problem and new tests".

Comment 4 Hao Zhang

2010-05-04 23:56:08 EDT

Chris, I've reviewed on this patch. It's good in all aspects.

Comment 5 Chris Goldthorpe

2010-05-05 12:27:50 EDT

Patch committed, Fixed