This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
There are some settings which are defined in User layer after changing them and these settings are not reset after action Revert Def. on Default layer. Settings are only reset after restart of IDE.
Probably the same problem arises when reverting changed settings from Project layer to User layer, but I cannot find such settings node, which is defined by default in User layer.
This problem affects SystemOptions, which doen't specify isGlobal()==false in their beaninfos. All SystemOptions are singletons, having just one instace in the IDE, their deserialization is written to update singleton instance by new data from the stream. Thus everything works good when .settings files contains serialized data (e.g. when switching between Project and User layer), unfortunately this isn't the case for XML layers holding the option's class only (default instance is created using ctor). When reverting definition of SystemOption to its default (either by using combobox in Tools/Optoins or by switching between projects) the option is not correctly reset.
Milane I'm going to attach patches for trunk and 3.3.1. Can you test them please?
Created attachment 5185 [details] Here is the patch for 3.3.1 distribution (lib/patches).
Created attachment 5186 [details] Here is the patch for dev (trunk) distribution (lib/patches).
Patches verified as working (the bug cannot be reproduced with patches) on linux and Solaris on NetBeans trunk build 200203260100 and Forte for Java build 020326.
fixed in the trunk and marked as 3.3.2_CANDIDATE. Vito, can you review the change please? http://openide.netbeans.org/source/browse/openide/src/org/openide/util /SharedClassObject.java.diff?r1=1.48&r2=1.49
seems good, go ahead with integration procedure
I had to reverted the fix to 1.48 revision due to performance regression. The fix extends startup time about 2-3s. There is not any other way how to simply fix the bug so I would suggest to do not include the fix to 3.3.2. Also decreased priority to P3 because it doesn't seem to break functionality of the ide significantly. Next step could be implementation of a persistent cache or using new (xml) persistence format for system option defining defaults already in the module layer.
*** Issue 23299 has been marked as a duplicate of this issue. ***
*** Issue 25422 has been marked as a duplicate of this issue. ***
Set target milestone to TBD
I've passed some code trying to solve the issue to Radek Matous who will incorporate it as a part of the new options ui stuff.
Jan (or Radek), could you please for the record attach the suggested code to this issue. I'm new maintainer and I would like to look at it. Thx.
Sorry, I removed those sources out of my machine as I had sent them to Radek. Anyway, the point of the patch was to store newly created SystemOption's instance into memory before it was deserialized already in SerialDataConvertor not in SCO. But drawback of such a solution is always a performance decrease. Even though it was not so bad in this case. IMO this issue relates to the issue #29702. You can read there my suggestion how to get around the problem with placing defaults of singeltons to a module layer.
the setting are reset. The only property sheet is not correctly (click to the another node in options and back to the changed node), isn't it?
Does the problem still persist? The last message by pzajac seems to be a verification of the problem being gone, is it not?
Does not work for me on Ant Settings, if I change Verbosity to Quiet and then reset it to the default layer. Even after clicking another node and back, still says Quiet (not Normal, the default value). Radio button stays on in "Default" column. If I manually reset verb. to Normal, marked as "User".
I have written a simulation test that works fine on integers: openide/test/unit/src/org/openide/loaders/InstanceDataObjectTest.java,v <-- InstanceDataObjectTest.java new revision: 1.32; However the actual problem with SystemOptions is still not addressed as the definition file for the system option does not contain the input stream to be read. I might solve this by clearing the property cache of the system option or by adding a reset method into SharedClassObject and let any subclass implement it themselves.
I have a fix. Each SystemOption can declare special method void reset() which will be called by SerialDataConvertor to initialize the setting when the .settings file contains just <instance .../> element. The change in implementation is not big as the SerialDataConvertor was already calling into the SharedClassObject.reset() method. I just change the call to call into all reset methods, even in subclasses. Btw. there is a lot of garbage related to isProjectOption() hacks in the SharedClassObject which I would rather delete then keep working. But the list of objects relying on isGlobal is rather long: ./editor/src/org/netbeans/modules/editor/options/AllOptions.java: public boolean isGlobal() { ./editor/src/org/netbeans/modules/editor/options/OptionSupport.java: public boolean isGlobal() { ./httpserver/src/org/netbeans/modules/httpserver/HttpServerSettings.java: private boolean isGlobal() { ./httpserver/httpserver4/src/org/netbeans/modules/httpserver/HttpServerSettings.java: private boolean isGlobal() { ./java/src/org/netbeans/modules/java/settings/JavaSettings.java: public boolean isGlobal() { ./java/src/org/netbeans/modules/java/settings/JavaSynchronizationSettings.java: public boolean isGlobal() { ./javadoc/src/org/netbeans/modules/javadoc/settings/DocumentationSettings.java: public boolean isGlobal() { ./utilities/src/org/netbeans/modules/search/SearchProjectSettings.java: public boolean isGlobal() { ./jndi/src/org/netbeans/modules/jndi/settings/JndiSystemOption.java: public final boolean isGlobal(){ ./rmi/src/org/netbeans/modules/rmi/settings/RMIActivationSettings.java: public boolean isGlobal() { ./rmi/src/org/netbeans/modules/rmi/settings/RMIRegistryItems.java: public boolean isGlobal() { ./rmi/src/org/netbeans/modules/rmi/settings/RMIRegistrySettings.java: public boolean isGlobal() { ./corba/src/org/netbeans/modules/corba/settings/CORBASupportSettings.java: public boolean isGlobal () { ./autoupdate/src/org/netbeans/modules/autoupdate/ResultListCellRenderer.java: boolean isGlobalCheckBoxSelected(java.awt.event.MouseEvent evt) { ./beans/src/org/netbeans/modules/beans/PropertyActionSettings.java: public boolean isGlobal() { ./xml/css/src/org/netbeans/modules/css/settings/CSSSettings.java: public boolean isGlobal () { ./jarpackager/src/org/netbeans/modules/jarpackager/options/JarPackagerOption.java: private boolean isGlobal () { ./debuggertools/src/org/netbeans/modules/debugger/debug/ToolsDebuggerProjectSettings.java: public boolean isGlobal() { ./jasm/src/org/netbeans/modules/jasm/settings/JASMSettings.java: public boolean isGlobal () { ./jini/src/org/netbeans/modules/jini/settings/JiniSettings.java: public boolean isGlobal() { ./junit/src/org/netbeans/modules/junit/JUnitSettings.java: public boolean isGlobal() { ./junit/test/function/src/org/netbeans/modules/junit/JUnitSettingsTest.java: /** Test of isGlobal method, of class org.netbeans.modules.junit.JUnitSettings. */ ./junit/test/function/src/org/netbeans/modules/junit/JUnitSettingsTest.java: assertTrue(false == JUnitSettings.getDefault().isGlobal()); ./objectbrowser/src/org/netbeans/modules/objectbrowser/ObjectBrowserSettings.java: public boolean isGlobal() { ./objectbrowser/src/org/netbeans/modules/objectbrowser/ObjectBrowserSettings.java:* 6 Gandalf 1.5 1/14/00 Radko Najman isGlobal() method added ./wasp/wasp10/src/com/idoox/nbm/wasptools/wasp/settings/SOAPSettings.java: public boolean isGlobal() { ./wasp/wasp10/src/com/idoox/nbm/wasptools/wasp/warpackager/options/WarPackagerOption.java: private boolean isGlobal () { ./openide/src/org/openide/util/SharedClassObject.java: static final String GLOBAL_METHOD_NAME = "isGlobal"; // NOI18N ./openide/src/org/openide/util/SharedClassObject.java: // the old hack with undocumented method isGlobal As concern this bug, I guess we have few choices: 1. lower the priority 2. make AntSettings use isGlobal, if possible 3. apply my patch (plus arch-core-settings, core/settings api change and not in SharedClassObject or SystemOption javadoc) 4. #3 plus remove the isGlobal hack I vote for #3 for present time. With an option to do #4 in future. Please react soon otherwise we will not be able to fix this for 4.0.
Prefer #3 or #4. Does this mean each system option is going to have to implement reset() itself? That is a big change. (I don't care so much about Ant Settings in particular, it was just one example that was handy.) Maybe we should close as WONTFIX and just wait for Registry to deal with this kind of problem more gracefully. SharedClassObject is far more trouble than it is worth, clearly.
I think #3 seems to be the best choice for now. It is simple and gives a chance to fix the bug for options where the problem is most visible. And actually does not require so much changes. WONTFIX is also worth thinking - because the fix is not perfect - the decision depends on how serious this issue actually is.
The chance to fix a problem when it is visible is imho best on this fix and that is the reason I like it. Unless somebody lowers the priority we are going to integrate the fix (with the documentation updates mentioned above) on Friday.
Just a note: maybe there isn't necessary to add new reset method into SystemOption but rather make SharedClassObject.reset() protected instead of private (already called from XMLSettingsSuppor).
That wouldn't help XMLSettingsSupport - it calls it as public. You would have to make the method public. We could probably keep it final, though, right? Get rid of the useless isProjectOption() method (pretend it is always true). Then make SCO.reset() a public final method, and have XMLSettingsSupport call it normally. Would I think work for all system options with no need to fix each one individually. Right? Even better, get rid of the system option hack and make SCO.reset nonfinal, with an empty default body; have SystemOption override it to be final and to do what it does now for sys opts.
I have evaluated your comments and decided to reimplement my original proposal. I am going to present it into three different versions, so my dear audience can express its preferences. My goal was to minimize the amount of changed modules. So in my first rewrite I decided to _not_ do anything in core/settings. Just leave them as they are. In order to achieve that I needed a protected SharedClassObject.reset() and of course the change in ant module. I could make the method public, but then people would call it, which is not exactly what we want. I can get rid of the reflection in core/settings if required, but as I wrote, I wanted as less changed modules as possible. Because I agree with Jesse that the isProjectOption is something that we should get rid of, I have created a second patch, that includes most of the first one but leaves the SharedClassObject.reset() method empty. Again it is subclassed in ant module. Then I thought about the issue a bit and I decided to minimize the amount of changed modules just to openide. The advantage is that no correctly written SystemOption needs to do anything and it will be resetable. This is due to the default implementation of SystemOption.reset in the third patch. Given the fact that I am now proposing three patches (well, the last one is my favourite) and that there may be small bugs in the implementation, I am going to wait with integration to hear your opinions and also to _not_ include this into beta2. I guess this issue is not that hot anyway.
Created attachment 17322 [details] Introduces protected SCO.reset(), does not need any changes in core/settings
Created attachment 17323 [details] Same as previous one, but also removes the old projects hacks that tried to serialize the option on instantiation
Created attachment 17324 [details] Adds SystemOption.reset() that works well for options using putProperty(String,Object,true) thus removes the need to change AntSettings
I agree that the 3rd patch looks best. I think the simplest patch - and one that would not require any API changes - is to just delete SCO.isProjectOption and make SCO.reset not check iPO at all. Wouldn't that suffice?
I am affraid of performance regressions (they happened before see the comments above) if we enabled the serialization of each option during startup. I am glad I could get rid of that code completely and replace it by the PROP_ORIGINAL_VALUES map.
Makes sense, I didn't realize there were performance problems with the ser/deser trick.
Thanks for your review.
cvs -q ci -m "#20962: SharedClassObject.reset made protected and correctly implemented in SystemOption so the tools options dialog can reset system options to their default values. Of course subclasses can always implement the reset in a way that is the most appropriate for them." Checking in openide-spec-vers.properties; /cvs/openide/openide-spec-vers.properties,v <-- openide-spec-vers.properties new revision: 1.156; previous revision: 1.155 done Processing log script arguments... More commits to come... Checking in api/doc/changes/apichanges.xml; /cvs/openide/api/doc/changes/apichanges.xml,v <-- apichanges.xml new revision: 1.218; previous revision: 1.217 done Processing log script arguments... More commits to come... Checking in src/org/openide/options/SystemOption.java; /cvs/openide/src/org/openide/options/SystemOption.java,v <-- SystemOption.java new revision: 1.37; previous revision: 1.36 done Processing log script arguments... More commits to come... Checking in src/org/openide/util/SharedClassObject.java; /cvs/openide/src/org/openide/util/SharedClassObject.java,v <-- SharedClassObject.java new revision: 1.65; previous revision: 1.64 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/loaders/InstanceDataObjectTest.java; /cvs/openide/test/unit/src/org/openide/loaders/InstanceDataObjectTest.java,v <-- InstanceDataObjectTest.java new revision: 1.34; previous revision: 1.33 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/options/SystemOptionTest.java; /cvs/openide/test/unit/src/org/openide/options/SystemOptionTest.java,v <-- SystemOptionTest.java new revision: 1.4; previous revision: 1.3
OK, bug is closed but we still have isGlobal() methods defined in our options throughout codebase. Can we remove them now (SharedClassObject.GLOBAL_METHOD_NAME seems to be unused)? Any issue to track this?
Yes, we should remove the obsolete isGlobal methods. Please file a fresh issue to track it.
I filled issue 49047
[dev 20041031, JDK 1.5.0_01, WinXP] Doesn't seem to be working the same way for all settings. I tried settings under node Editing in Tools|Options and it works weird. I changed e.g. settings of Internationalization node (checked two checkboxes), green mark moved to User layer, I clicked the blank circle in Default layer and selected "Revert ..." - green mark moved but settings were not changed. Then I did the same with difference that I unchecked those checkboxes, did "Revert ..." and setttings were changed (checkboxes changed to checked). After that it still worked this way - but the "Revert" action didn't really change to default values.
Created attachment 18732 [details] Proposed enhancement of current 4.0 state with one api change
You are right, in certain situations the fix does not work. Every property that is using style like I18NOptions: public String getI18nRegularExpression() { // Lazy init. if(getProperty(PROP_I18N_REGULAR_EXPRESSION) == null) return (String)I18nUtil.getI18nRegExpItems().get(0); return (String)getProperty(PROP_I18N_REGULAR_EXPRESSION); } can be broken. I've managed to fix it, but I need an api between SystemOption and SharedClassObject to find out that initialize is being called. I using special getProperty (...) for that. Reviewers please express your opinion. Btw. the fix will likely need to wait for 4.1 (if not important enough).
4.1 sounds more appropriate at this point. BTW testTheProblemWithI18NOptions is a cool method name. :-) Tip: mention this issue # in a comment in the test method so future readers will have some clue why this is being tested...
Will apply the fix tomorrow. All the tests in the class are related to this issue, the new test is being added because this issue has been reopened. Adding comment to it to point here would not be quite correct as other tests should have it as well.
Checking in arch/arch-openide-settings.xml; /cvs/openide/arch/arch-openide-settings.xml,v <-- arch-openide-settings.xml new revision: 1.19; previous revision: 1.18 done Checking in arch/arch-openide-utilities.xml; /cvs/openide/arch/arch-openide-utilities.xml,v <-- arch-openide-utilities.xml new revision: 1.16; previous revision: 1.15 done Processing log script arguments... More commits to come... Checking in src/org/openide/options/SystemOption.java; /cvs/openide/src/org/openide/options/SystemOption.java,v <-- SystemOption.java new revision: 1.38; previous revision: 1.37 done Processing log script arguments... More commits to come... Checking in src/org/openide/util/SharedClassObject.java; /cvs/openide/src/org/openide/util/SharedClassObject.java,v <-- SharedClassObject.java new revision: 1.67; previous revision: 1.66 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/options/SystemOptionTest.java; /cvs/openide/test/unit/src/org/openide/options/SystemOptionTest.java,v <-- SystemOptionTest.java new revision: 1.5; previous revision: 1.4
Verified in dev-200501191900.