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.
jdk-1.5 linux daily 20040323 1. open a java file (i haven't checked other types) 2. modify the file. the dirty flag (asterisk on tab) shows up. this is ok. 3. save file. dirty flag goes away. this is ok. 4! press undo (ctrl-z). changes are undone but file is not marked as modified. this is a P1 because someone may unwittingly exit without saving (since the IDE doesn't think the file is dirty, it won't prompt to save changes). in fact, the IDE will actually PREVENT you from saving since the dirty flag is not set. it's impossible to tell if redo has the same problem until undo is fixed.
I was able to reproduce the bug. It can lead to dataloss, indeed. Please fix ASAP.
Looks like the issue 56834 describes the similar if not the same problem. I'll leave both of them open for now.
Well, I probably know what's wrong. Due to the fix of issue 51872 it's now necessary to do some special treatment of the edits which is probably not done appropriately during undo. I'll investigate more.
BTW I've tested daily build 20050314 (prior issue 51872 integration) and it works fine. 51872 was integrated on 2005/03/16. This issue can be reproduced since build 20050317 so it's likely caused by 51872 integration.
*** Issue 57075 has been marked as a duplicate of this issue. ***
Well, it's clear why it fails. The DocumentEvent.undo() just does undo of its contained edits and fires document listener. Since the notifyModified() no longer watches the DocumentListener but instead has its own treatment it will ignore the firing of the listener and the document will remain unmodified. I'll try to fix on my side. We should certainly add a test for this case on openide/editor side as well.
Created attachment 21200 [details] Possible fix for the editor - still needs some additional fix on openide/editor side
I've made the possible fix for the editor but it seems that it still needs some additional fixing on openide/editor side. I have been trying to debug the processing in CloneableEditorSupport but it's rather complicated due to all the present fixes including the one for issue 51872. Yarda's help would be much appreciated.
*** Issue 57054 has been marked as a duplicate of this issue. ***
I've done additional change on editor side that seems to fix the problem completely. After testing I'll commit it.
Fixed in main trunk: Checking in libsrc/org/netbeans/editor/BaseDocument.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocument.java,v <-- BaseDocument.java new revision: 1.118; previous revision: 1.117 done Checking in libsrc/org/netbeans/editor/BaseDocumentEvent.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java,v <-- BaseDocumentEvent.java new revision: 1.21; previous revision: 1.20
The following case fails: 1. Open any file in editor. 2. Select all contents. 3. Press delete. 4. Save file. 5. Undo changes. After step 5 you are not able to save the file. Thus you may loose the whole content of the file if you loose your clipboard. It works fine in 4.0 so it is a regression.
I'll investigate.
Let's try to write a test tomorrow.
Created attachment 21325 [details] Test that works without any changes in openide
I've just attached test for the broken behaviour. It works correctly with plain NotifyModifiedTest and after small change in NbLikeEditorKit also with NotifyModifiedOnNbEditorLikeKitTest. The problem is that when doing undo on AbstractDocument the StringContent changes without calling AbstractDocument.insertString or AbstractDocument.remove. I had to override fire*** methods to be notified of such changes. Right now my version (a week outdated) of InheritedNotifyModifiedTest fails, but I guess that similiar fix as I used in NbLikeEditorKit can be used in editor as well. Please try it and if it works please integrate my test at least to trunk together with your fix.
Yardo, I've already did the same sort of changes in the editor module in BaseDocument and BaseDocumentEvent. Maybe my ones are slightly more elaborated ;) because if I understand your changes correctly you just increment the changes variable in fire* methods and rely that the undo/redo changes are invoked under runAtomic() which then fires the modificationListener (which is true thanks to the fact that CESUndoRedoManager wraps each operation with RenderUndo). My undo/redo impl would work even without being wrapped runAtomic(). But that's just a nitpick. I've found what's the problem with the first undo - the inUndoRedo flag passed to BaseDocument.atomicUnlockImpl() was incorrectly set to false for the first undo so I've fixed that by using another way of marking that there were any edits undone or redone. Your extra test then passes with the BaseDocument implementation but then I've found that once I do redo the document content is deleted properly but the file stays modified which is an error. So I've added the redo() test into your extra test but then the test deadlocks every time (at least on my machine). I'm attaching my BaseDocument changes and the modified openide test and the deadlock dump.
Created attachment 21346 [details] Openide tests diff
Created attachment 21347 [details] Editor fix
Created attachment 21348 [details] Deadlock during added redo in the test
Created attachment 21350 [details] Fulldiff with yet another test for reload - all tests are now passing fine
Yardo, one more thing that fails: 1) file open 2) any modification 3) undo 4) file is still marked dirty Test method (now it deadlocks in a similar way like the previous one): public void testUndoMarksFileUnmodified () throws Exception { content = "Somecontent"; final javax.swing.text.StyledDocument doc = support.openDocument(); int len = doc.getLength (); assertEquals ("Content opened", "Somecontent", doc.getText (0, len)); doc.remove (0, len); assertTrue ("Document is modified", support.isModified ()); assertTrue ("Can undo", support.getUndoRedo ().canUndo ()); support.getUndoRedo ().undo (); assertTrue ("Document is unmodified", !support.isModified ()); }
"#56963: Four new ways how to break yesterday's undo/redo support in CloneableEditorSupport automated and fixed. Works fine for all nb-like editor kits, however has intermitent problems with non-nb-like ones. But as the fix for nb-lie ones is more important I am integrating to trunk anyway to reach wider audience and get feed back. Either confirmation that it works or new ways to still break the support" Checking in openide/src/org/openide/text/CloneableEditorSupport.java; /cvs/openide/src/org/openide/text/CloneableEditorSupport.java,v <-- CloneableEditorSupport.java new revision: 1.151; previous revision: 1.150 done Checking in openide/test/unit/src/org/openide/text/NbLikeEditorKit.java; /cvs/openide/test/unit/src/org/openide/text/NbLikeEditorKit.java,v <-- NbLikeEditorKit.java new revision: 1.5; previous revision: 1.4 done Checking in openide/test/unit/src/org/openide/text/NotifyModifiedTest.java; /cvs/openide/test/unit/src/org/openide/text/NotifyModifiedTest.java,v <-- NotifyModifiedTest.java new revision: 1.7; previous revision: 1.6 done Checking in editor/libsrc/org/netbeans/editor/BaseDocument.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocument.java,v <-- BaseDocument.java new revision: 1.120; previous revision: 1.119 done Checking in editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java,v <-- BaseDocumentEvent.java new revision: 1.23; previous revision: 1.22
Shouldn't this be marked with 41_HR_FIX kw?
Yes, I've added the keyword. I would like to ask Petr N. to check and approve the fix. For me the fix looks fine.
The changes in CloneableEditorSupport.java look sane and complete to me. I won't pretend I deeply understand BaseDocument impl but the changes look OK to me. Any more reviewer able to check BaseDocument deeper?
Forgot to add diff urls: http://openide.netbeans.org/source/browse/openide/test/unit/src/org/openide/text/NbLikeEditorKit.java.diff?r1=1.4&r2=1.5 http://openide.netbeans.org/source/browse/openide/test/unit/src/org/openide/text/NotifyModifiedTest.java.diff?r1=1.6&r2=1.7 http://editor.netbeans.org/source/browse/editor/libsrc/org/netbeans/editor/BaseDocument.java.diff?r1=1.119&r2=1.120 http://editor.netbeans.org/source/browse/editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java.diff?r1=1.22&r2=1.23 Mato, could you please review the BaseDocument part? Thanks.
Yes, the changes in BaseDocument and BaseDocumentEvent seem fine to me. The logic of the change is clear. (But as for the global mechanism of locking, notifying and firing - Mila, Jarda, it's up to you :-) )
I've tested the issue on build 200504061800, it works well here. Didn't see any case where the dirty mark would be missing. Please procceed with integration into 4.1 branch. Thanks.
"#57104,#56963: Backport of undo/redo/save fixes in editor" Checking in openide/src/org/openide/text/CloneableEditorSupport.java; /cvs/openide/src/org/openide/text/CloneableEditorSupport.java,v <-- CloneableEditorSupport.java new revision: 1.150.2.1; previous revision: 1.150 done Checking in openide/test/unit/src/org/openide/text/NbLikeEditorKit.java; /cvs/openide/test/unit/src/org/openide/text/NbLikeEditorKit.java,v <-- NbLikeEditorKit.java new revision: 1.4.4.1; previous revision: 1.4 done Checking in openide/test/unit/src/org/openide/text/NotifyModifiedTest.java; /cvs/openide/test/unit/src/org/openide/text/NotifyModifiedTest.java,v <-- NotifyModifiedTest.java new revision: 1.6.4.1; previous revision: 1.6 done Checking in editor/libsrc/org/netbeans/editor/BaseDocument.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocument.java,v <-- BaseDocument.java new revision: 1.119.2.1; previous revision: 1.119 done Checking in editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java; /cvs/editor/libsrc/org/netbeans/editor/BaseDocumentEvent.java,v <-- BaseDocumentEvent.java new revision: 1.21.2.1; previous revision: 1.21
Setting TM to 4.1 as the fix was backported to 4.1 branch. See: http://www.netbeans.org/community/releases/41/high-resistance.html
Verified in 200507131800.