Bugzilla – Full Text Bug Listing
| Summary: | [dstore] Customization of connection timeout does not work | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Target Management | Reporter: | Masao Nishimoto <e03616> | ||||||||||
| Component: | RSE | Assignee: | David McKnight <dmcknigh> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||||||||
| Severity: | major | ||||||||||||
| Priority: | P3 | CC: | kjdoyle, xuanchen | ||||||||||
| Version: | 3.0 | Flags: | mober.at+eclipse:
pmc_approved+
kjdoyle: review+ dmcknigh: review+ xuanchen: review+ |
||||||||||
| Target Milestone: | 3.0.1 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows XP | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Masao Nishimoto
Are you absolutely sure that this is an issue? My experience (with the SHOW_EMPTY_LISTS Preference) is that the plugin_customization.ini method works properly even if store.setDefault() is executed. I'm not exactly sure why this is the case (maybe because SHOW_EMPTY_LISTS is not initialized from the Activator.start() method but from SystemPreferenceInitializer.initializeDefaultPreferences() which implements the org.eclipse.core.runtime.preferences extension point). Perhaps that extension is executed *before* the plugin_customization.ini Preferences are applied such that plugin_customization.ini can override the hardcoded default-defaults. I also have the following line in the plugin_customization.ini, and it works. org.eclipse.rse.ui/org.eclipse.rse.preferences.shownewconnection=true I confirmed that the setting in the plugin_customization.ini is in effect, not when SystemPreferenceInitializer.initializeDefaultPreferences() is executed, but when Activator.initializeDefaultPreferences() is executed. See also bug 245714 Created attachment 112903 [details]
here's a patch for this
I've tried this but it doesn't seem to work for me. Martin, am I missing something in this patch?
This is a major issue since I cannot think of a workaround. Being able to customize products is very important. Created attachment 112910 [details]
updated patch with hack to trigger intializer
Comment on attachment 112910 [details]
updated patch with hack to trigger intializer
this won't work with plugin_customization.ini because we're still using the RSE UI preferences
Created attachment 112920 [details]
Patch fixing the problem
The proposed patch does not fix the problem: The core.preferences extension can not be used here because the Preferences in question are actually stored in a different plugin. As a result, the following can happen:
1. RSEUIPlugin Preferences get accessed
2. plugin_customization Preferences are applied to RSEUIPlugin
3. Connectorservice.dstore gets loaded
4. Coded Defaults from there override the customized defaults
The correct order of initialization can be seen in class DefaultPreferences:
private void loadDefaults() {
applyRuntimeDefaults();
applyBundleDefaults();
applyProductDefaults();
applyCommandLineDefaults();
}
Since our code here is potentially loaded *after* defaults have been applied to RSEUIPlugin already, we must take care not to override any defaults that have been set already. The problem here is, that the IPreferenceStore API does not provide a method for checkign whether a default has been set already; the newer IPreferencesService = Platform.getPreferencesService() API must be used here.
I tested the patch and it works fine for me, and I propose re-building 3.0.1 to include this patch. I consider it low-risk since the normal path of operation is not changed in the normal case (default preference==null).
I'm approving my own patch for 3.0.1 rebuild. Dave and Kevin, what do you think? Patch committed for 3.1M2 -- Xuan what do you think about releasing this into a 3.0.1 rebuild? Created attachment 112928 [details]
Improved patch
Here is actually a better patch, that's less code and works without extending ScopedPreferenceStore. ScopedPreferenceStore is not marked @noextend, but extending an API class still bears some risk: In case a "hasDefault()" method would be added to ScopedPreferenceStore in the future, our code might break.
It's therefore safer to just call existing API methods rather than extending the class. Other than that, functionality is the same as the other patch (tested to work).
I am fine with putting this fix into 3.0.1. Thanks. The fix has been rolled into a TM 3.0.1 Rebuild: http://download.eclipse.org/dsdp/tm/downloads/drops/R-3.0.1-200809181400/ http://download.eclipse.org/dsdp/tm/updates/3.0.1milestones/ Please test and verify. Verified |