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

Bug 126705

Summary: [Preferences] FileFieldEditor does not call doCheckState
Product: [Eclipse Project] Platform Reporter: Michael Illgner <fillg1>
Component: UIAssignee: Oleg Besedin <ob1.eclipse>
Status: VERIFIED FIXED QA Contact: Oleg Besedin <ob1.eclipse>
Severity: normal    
Priority: P3 CC: mayank.kumar, ob1.eclipse, ttencate
Version: 3.1Keywords: helpwanted
Target Milestone: 3.6 M4   
Hardware: PC   
OS: Windows XP   
Whiteboard: hasPatch
Attachments:
Description Flags
Patch for FileFieldEditor.java
none
Fixed patch
ob1.eclipse: iplog+
Updated patch none

Description Michael Illgner CLA 2006-02-07 04:53:11 EST 
org.eclipse.jface.preference.FileFieldEditor does not call doCheckSate() during checkState() which makes it difficult to subclass this editor to perform additional checks like it is possible with StringFieldEditor
Comment 1 Mayank Kumar CLA 2008-09-13 11:14:20 EDT 
Created attachment 112495 [details]
Patch for FileFieldEditor.java

Proposed fix.
Comment 2 Mayank Kumar CLA 2008-09-13 12:49:59 EDT 
There's a problem with the previously submitted patch. Re-looking into it.
Comment 3 Mayank Kumar CLA 2008-09-13 13:42:06 EDT 
Created attachment 112498 [details]
Fixed patch

Patch for FileFieldEditor.java.

Gives precedence to the check status of FileFieldEditor. If an error is not found in the checkStatus of FileFieldEditor then doCheckStatus is invoked.
Comment 4 Boris Bokowski CLA 2009-05-06 16:48:42 EDT 
Removing 3.5 target milestone. We are in the end-game now. Please have a look and decide if this should be targeted at 3.6.
Comment 5 Oleg Besedin CLA 2009-05-08 11:00:17 EDT 
Sure, let's look it in the 3.6.
Comment 6 Thomas ten Cate CLA 2009-06-15 08:46:27 EDT 
I ran into this issue as well.

FileFieldEditor currently overrides checkState() and reimplements everything from its superclass StringFieldEditor (with a better implementation, I might add), *except* the call to doCheckState().

Instead, I think FileFieldEditor should *not* override checkState(), but rather give its own implementation of doCheckState(). That is exactly what doCheckState is intended for, isn't it?
Comment 7 Oleg Besedin CLA 2009-11-26 13:53:51 EST 
Created attachment 153200 [details]
Updated patch

(In reply to comment #6)
> Instead, I think FileFieldEditor should *not* override checkState(), but rather
> give its own implementation of doCheckState().

Yes, that would be best, but this is a public non-final class so no telling how people subclassed it.

I changed Mayank's patch a bit to account for a case where error message gets modified by a subclass.
Comment 8 Oleg Besedin CLA 2009-11-26 13:56:01 EST 
Updated patch applied to CVS Head. Thank you!
Comment 9 Oleg Besedin CLA 2009-12-08 10:41:54 EST 
Verified in I20091207-1800.