Bug 126705 - [Preferences] FileFieldEditor does not call doCheckState
Summary: [Preferences] FileFieldEditor does not call doCheckState
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M4   Edit
Assignee: Oleg Besedin CLA
QA Contact: Oleg Besedin CLA
URL:
Whiteboard: hasPatch
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2006-02-07 04:53 EST by Michael Illgner CLA
Modified: 2009-12-08 10:41 EST (History)
3 users (show)

See Also:

Attachments
Patch for FileFieldEditor.java (600 bytes, patch)
2008-09-13 11:14 EDT, Mayank Kumar CLA 		no flags Details | Diff
Fixed patch (739 bytes, patch)
2008-09-13 13:42 EDT, Mayank Kumar CLA 		ob1.eclipse: iplog+
Details | Diff
Updated patch (982 bytes, patch)
2009-11-26 13:53 EST, Oleg Besedin CLA 		no flags Details | Diff
Show Obsolete (1) View All 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 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.