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.

Bug 227270 - Save button is always active
Summary: Save button is always active
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Lookup (show other bugs)
Version: 7.3
Hardware: All All
: P2 normal (vote)
Assignee: Jaroslav Tulach
URL:
Keywords:
: 232975 235930 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-03-11 07:44 UTC by Vladimir Voskresensky
Modified: 2013-09-16 05:34 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Voskresensky 2013-03-11 07:44:52 UTC
I modified file, Save action became active in Toolbar.
Pressed it, but it is still active. each press cause two records in message log:
WARNING [org.netbeans.api.actions.Savable]: Savable class org.openide.loaders.DataObject$DOSavable is not in Savable.REGISTRY! Have not you forgotten to call register() in constructor?
WARNING [org.netbeans.api.actions.Savable]: Savable class org.openide.loaders.DataObject$DOSavable is not in Savable.REGISTRY! Have not you forgotten to call register() in constructor?

File's title is in bold as well => I assume content is not flushed to disk and my work can be lost.
Comment 1 Vladimir Voskresensky 2013-03-11 07:45:34 UTC
Product Version: NetBeans IDE Dev (Build 20130311-bb4aff7fbcf4)
Java: 1.7.0_17; Java HotSpot(TM) Server VM 23.7-b01
Runtime: Java(TM) SE Runtime Environment 1.7.0_17-b02
System: SunOS version 5.10 running on x86; UTF-8; ru_RU (nb)
Comment 2 Miloslav Metelka 2013-03-22 08:19:03 UTC
Reassigning to platform/actions for evaluation.
Comment 3 Jaroslav Tulach 2013-03-25 10:45:52 UTC
"I modified file" - what kind of file? Any?
Comment 4 Vladimir Voskresensky 2013-03-25 14:13:05 UTC
it was Java file
Comment 5 Vladimir Voskresensky 2013-04-08 07:57:19 UTC
The same issue is with XML file.
I did select all, Ctrl-X, Save, Ctrl+V => pressing Save button on toolbar stay active and messages in log:
 WARNING [org.netbeans.api.actions.Savable]: Savable class org.openide.loaders.DataObject$DOSavable is not in Savable.REGISTRY! Have not you forgotten to call register() in constructor?
WARNING [org.netbeans.api.actions.Savable]: Savable class org.openide.loaders.DataObject$DOSavable is not in Savable.REGISTRY! Have not you forgotten to call register() in constructor?
Comment 6 Vladimir Voskresensky 2013-04-08 07:58:02 UTC
Btw, after changing focus from IDE and back to IDE => I was able to press Save in toolbar and it became inactive
Comment 7 Jan Peska 2013-07-11 10:48:03 UTC
Vladimir, are you able to reproduce it?
Comment 8 Jan Peska 2013-07-11 12:51:20 UTC
It seems sometimes DataObject.setModified is not called and as a result DataObject.DOSavable does not register Savable to the Savable.REGISTRY. Unfortunately I was able to reproduce it only once so I don't know what could cause it but think DataEditorSupport.markModified() is not called so I'm reassigning this issue to Editor for further evaluation.
Comment 9 Svata Dedic 2013-07-15 07:17:54 UTC
Reproduced with the following scenario:

* open an XML file
* select all
* Ctrl-X
* mouse-click the 'save' button

Sometimes the error shows up during this save operation. If not, then
* press Undo
* mouse-click the 'save' button

If Ctrl-S is pressed for save, the system works correctly and save button is disabled after the save completes. To get the error, toolbar button must be used.

Re.: focus change: mere change in focus in TopComponents, e.g. switch to another editor, then back, without leaving NetBeans causes the action to behave well. I suspect EditorRegistry.lastFocusedComponent is somehow involved.
Comment 10 Svata Dedic 2013-07-15 13:35:31 UTC
According to my debugging, the following happens:

AbstractSavable.save() constructs a Lookup.Template with 'this' as an instance, and tries to lookup all Savables using this template. 

DelegatingStorage.findResult() attempts to locate a cached result for the template, and uses template.equals() to compare the known and queries templates. Lookup.Template.equals() compares instances (if they are known) using equals().

In the debugged situation, the cached result contained a different instance of DOSavable than specified in the template, yet DOSavable defines equals so that DOSavables are equal iff their DataObjects are equal - and in my debugging session, both Savables referred to the same instance of DO. So the cached Template is used as a reference in the returned AbstractLookup.R.

Later in Lookup.Result.allInstances() implementation, in AbstractLookup.R.initItems(), the item to be returned is compared for match with the reference (cached ?) template. The comparison in match() method compares instances using == operator. However Storage.lookup() yields the current DOSavable, so it does not match the cached template.

IMHO both template matches should be done either using == or equals, or the pre-selection should use more strict (==) comparison. As the implementation stands, an invalid ReferenceToResult is selected from the cache, and is eliminated later.

The observations on enabling/disabling the button actually depends on how the caches are used, how the DOSavables are (un)registered and when the lookup caches expire.

To inspect, stop the debugger in AbstractSavable.save(), on the condition savables.isEmpty() and try to execute the scenario I've written in the previous comment. I actually made this save() method synchronize with Savable.REGISTRY.register() and unregister() so that other threads cannot interfere with debugging
Comment 11 Jaroslav Tulach 2013-07-17 11:57:20 UTC
Thanks for the evaluation Sváťo, I try to write unit test to narrow the problem in lookup down. Have you tried to change the code to use == or equals and see if the enablement issue goes away?
Comment 12 Svata Dedic 2013-07-17 12:10:00 UTC
I admit it might seem as laziness, but I was simply not able to evaluate/guess consequences of behaviour change in Lookup, the caching code was quite a puzzle for me - sorry.
Comment 13 Jaroslav Tulach 2013-07-17 12:59:41 UTC
ergonomics#ec3d638a6599
Comment 14 soldatov 2013-07-18 11:11:26 UTC
*** Bug 232975 has been marked as a duplicate of this bug. ***
Comment 15 Quality Engineering 2013-07-20 02:11:53 UTC
Integrated into 'main-silver', will be available in build *201307192300* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/ec3d638a6599
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #227270: Use equals when comparing created instances
Comment 16 Milos Kleint 2013-09-16 05:34:45 UTC
*** Bug 235930 has been marked as a duplicate of this bug. ***