Bug 205835 - Closing editor clone not handled correctly; editor TC out of sync with disk file
Closing editor clone not handled correctly; editor TC out of sync with disk file
Status: VERIFIED FIXED
Product: platform
Classification: Unclassified
Component: Window System
7.1
All All
: P1 with 7 votes (vote)
: 7.1.1
Assigned To: Jaroslav Tulach
issues@platform
71patch1-verified
: REGRESSION, RELNOTE
: 206295 207135 207973 208598 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-02 10:58 UTC by err
Modified: 2012-03-01 16:20 UTC (History)
15 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
a crude patch (1.55 KB, patch)
2011-12-07 21:36 UTC, Stanislav Aubrecht
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description err 2011-12-02 10:58:51 UTC
To reproduce, have file open: foo.java
- clone foo.java (now two editors on foo.java open)
- add some chars to foo.java
- "X" (close) foo.java editor TC just modified
Observe
    Dialog Save/Discard/Cancel
        I believe the DIALOG SHOULD NOT BE SHOWN
        if there is another open editor on the same file. 
        The EDITOR TC SHOULD JUST CLOSE.
        (Alternately could reload foo.java in other editors,
        but this feels wrong since there is no way to close
        the extra TC without either discarding or saving changes in progress.)
- Select Discard
Observe inconsistency with the other foo.java open
    * other foo.java TC still shows text changes (OUCH)
      but tab does not show modified
    * save/undo/redo toolbar buttons are disabled
    * mercurial>diff of open editor foo.java says no local changes
Comment 1 err 2011-12-02 11:00:58 UTC
Mark as regression. (Leave priority as P3, but tempted to make P2)
Comment 2 err 2011-12-02 11:53:53 UTC
Changing to P2 (can't get it out of my head); probably goes back to P3 but...

Forgot to include:
Product Version: NetBeans IDE 7.1 RC1 (Build 201111242103)
Java: 1.6.0_23; Java HotSpot(TM) Client VM 19.0-b09
System: Windows XP version 5.1 running on x86; Cp1252; en_US (nb)
Comment 3 err 2011-12-03 19:35:37 UTC
This is worse than I thought.

If you do
- clone editor
- close the clone
Observe that the original is now "unhooked" from the disk file. You can modify the text in the editor, but undo/redo/save stay disabled. 

Close the editor and any changes are lost
Comment 4 limo42 2011-12-06 12:54:42 UTC
Comment 3 is correct, you're losing all changes done to the clone after the original has been closed. BTW, indenting, error badges, etc. stop working on the
clone as well. Maybe the description sounds a bit too innoncent.

This is data loss without workaround, clearly meaning P1 as per the guidelines in the wiki. I'm increasing priority to boost the motivation to have a look at it. This is a stopper for 7.1.
Running NetBeans 7.1rc1 on Linux and JDK 1.6.0_29, so it isn't windows-only either.
Comment 5 Jiri Prox 2011-12-06 14:31:04 UTC
reproducible
Comment 6 Miloslav Metelka 2011-12-07 13:12:06 UTC
MultiViewCloneableEditor.canCloseElement() only finds one item in the following enumeration:

        Enumeration en = getReference().getComponents();
        if (en.hasMoreElements()) {
            en.nextElement();
            if (en.hasMoreElements()) {
                // at least two is OK
                return CloseOperationState.STATE_OK;
            }
        }

It later goes into MultiViewPeer.canClose() where

            return closeHandler.resolveCloseOperation(states);

displays the dialog asking for save.
Reassigning to platform/window-system since it's multi-view related.
Comment 7 Stanislav Aubrecht 2011-12-07 21:34:36 UTC
the problem was introduced in the new multiview support. the multiview editors don't know they have clones so closing a cloned multiview uninitializes all inner editors as if the last cloned multiview has been closed.
Comment 8 Stanislav Aubrecht 2011-12-07 21:36:52 UTC
Created attachment 113938 [details]
a crude patch

the attached patch does fix the problem however it would also break the cloning feature for non-multiview windows.
Comment 9 Marian Mirilovic 2011-12-08 01:00:18 UTC
I do not consider this as a stopper for 7.1 - cloning editor, closing the original and continue editing in cloned is used very rarely (this is 1st report since the multiview editors were integrated).

We will fix that into NB 7.1 Patch 1(7.1.1) - planned to be released Jan/Feb 2012. Note about this issue will be added to Release Notes for 7.1
Comment 10 Jeffrey Rubinoff 2011-12-08 10:28:02 UTC
Alyona handles release notes.
Comment 11 anba 2011-12-08 11:09:48 UTC
(In reply to comment #9)
> I do not consider this as a stopper for 7.1 

I do definitely not agree. If a data loss is not a show stopper, what else 
would be ? Editing, compiling, and storing source code are the main goals of an IDE and they must be absolutely reliable. Nobody will tolerate it if he / she loose the code written in the last hours and this bug has the potential for this (and it happened as a minimum 3 times that I know about, 2 shown here in the report and one time in our lab). Such a bug can people make to switch the IDE (and there are other IDEs on the market).

Only writing a note into the release notes is not sufficient. My own experience with software shows that nearly nobody reads release notes or documentation. I never did it when getting a new netbeans release. 

What are the alternatives:
1. Fix the bug for 7.1 
2. Remove the clone operation for 7.1 in the editor and add it again if the bug is fixed (maybe in 7.1.1).

I would prefer the first alternative because I use different views of the same file quite often.
Comment 12 limo42 2011-12-08 13:14:26 UTC
(In reply to comment #9)
> I do not consider this as a stopper for 7.1 - cloning editor, closing the
> original and continue editing in cloned is used very rarely (this is 1st report
> since the multiview editors were integrated).

The use case is different: editing a file, using a clone to refactor or move stuff around, then closing one of the editors and continuing the work in the
other. It doesn't matter whether the original or the clone is closed (speaking of clones, how could you tell which is which anyway?), the remaining editor stops working. 

I'm doing this daily, and the threat of data loss is just unacceptable.

> We will fix that into NB 7.1 Patch 1(7.1.1) - planned to be released Jan/Feb
> 2012. Note about this issue will be added to Release Notes for 7.1

This regression has been introduced with adding multiview to the standard editors
for 7.1. You should rather back this change out again until 7.1.1; I can always open a history view using the context menu.
Comment 13 Marian Mirilovic 2011-12-08 13:45:21 UTC
Ok guys, so I do agree :
- it's regression in 7.1
- it's high priority issue due data loss 

... and now why it will not be fixed into 7.1 :
- first and only one report since MV was integrated (5 months ago)
- we do not have a fix yet
- we are just about to release 7.1 (no time)

As I already said, this issue will be fixed in NB 7.1.1 (patch 1 for NB 7.1) ... in 6-8 weeks since release of 7.1 .
Comment 14 err 2011-12-08 15:34:53 UTC
The simple patch that saubrecht attached, comment #8, fixes the data loss issue and gives expected behavior for the typical case.

What is the impact of "break the cloning feature for non-multiview windows"?

In question is how much data loss is acceptable. The current "clone" operation is poison and it's use is almost guaranteed to cause some data loss. I experienced data loss more that once. It had to happen a few times before I tracked down the cause. I'll frequently view two areas of the same file.

The patch may minimize the negative impact during the period between 7.1 and 7.1.1.
Comment 15 Stanislav Aubrecht 2011-12-08 16:09:11 UTC
(In reply to comment #14)
> The simple patch that saubrecht attached, comment #8, fixes the data loss issue
> and gives expected behavior for the typical case.
> 
> What is the impact of "break the cloning feature for non-multiview windows"?
basically every cloneable TopComponent that isn't multiview will have this problem you've reported. there probably aren't that many such windows in the IDE now that everything is a multiview with history tab.
Comment 16 err 2011-12-08 16:33:23 UTC
> ... [with the patch] aren't that many such windows

That seems much more containable than putting out a general "looses your edits" (Merry Christmas) release.
Comment 17 AlyonaStashkova 2011-12-08 19:04:54 UTC
Jeff, thank you for cc-ing me.

The issue is added to the Core IDE section of the staged release notes and requires a review.
Comment 18 David Strupl 2011-12-09 17:15:42 UTC
I agree with Marian. I might suggest going even further: do we need the Clone functionality at all? Simply deleting it would solve the problem for good....
Comment 19 limo42 2011-12-09 17:23:32 UTC
(In reply to comment #18)
> I agree with Marian. I might suggest going even further: do we need the Clone
> functionality at all? Simply deleting it would solve the problem for good....

Ever had to refactor a large file using NetBeans? This is nearly impossible without cloning. Take away all the functionality we need and you'll be ending up with just a pathetic caricature of an IDE.
Comment 20 Marian Mirilovic 2011-12-10 01:37:26 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > I agree with Marian. I might suggest going even further: do we need the Clone
> > functionality at all? Simply deleting it would solve the problem for good....
> 
> Ever had to refactor a large file using NetBeans? This is nearly impossible
> without cloning. Take away all the functionality we need and you'll be ending
> up with just a pathetic caricature of an IDE.

Calm down all please, as I said we will fix it, but not for 7.1 ... wait for 7.1.1 or if you need it sooner: we can provide you a patched module(s) with the fix of this issue (once it's fixed) ?
Comment 21 err 2011-12-11 14:24:40 UTC
FYI
I'm currently checking for the presence of the bug with
            if (mi.getCodeNameBase().equals(
                    "org.openide.text")) {
                if (mi.getSpecificationVersion().compareTo(
                        new SpecificationVersion("6.40")) > 0
                    && mi.getSpecificationVersion().compareTo(
                        new SpecificationVersion("6.45")) < 0)
                {...}
            }
If the fix has org.openide.text with spec version >= 6.45 then my stuff should detect the fix and revert to expected behavior.
Comment 22 Jaroslav Tulach 2011-12-12 09:03:57 UTC
ergonomics#5a38e871f74e
Comment 23 Quality Engineering 2011-12-13 10:36:00 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/5a38e871f74e
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #205835: canClose yields true if there are multiple remaining cloneable multi view components. Only closeLast performs the close logic.
Comment 24 Marian Mirilovic 2011-12-13 11:49:30 UTC
*** Bug 206295 has been marked as a duplicate of this bug. ***
Comment 25 Marian Mirilovic 2012-01-10 08:58:30 UTC
err, anybody else ... could you please verify that this works in trunk without any problems ? If so we can proceed with integration into Patch 1 for 7.1. Thanks in advance.
Comment 26 err 2012-01-10 17:41:10 UTC
Verified that this problem is fixed; but see Bug 207136. (this fix may have introduced another issue)
Comment 27 Marian Mirilovic 2012-01-12 09:35:47 UTC
Yarda, 
could you please port the fix into release71_fixes branch As Soon As Possible ? We would like to provide this fix to 7.1 users in next 1-2 weeks. Also please check issue 207136 to be sure it's not a regression caused by this fix. Thanks in advance.
Comment 28 Jiri Prox 2012-01-13 10:55:16 UTC
*** Bug 207135 has been marked as a duplicate of this bug. ***
Comment 29 Jaroslav Tulach 2012-01-16 11:58:58 UTC
Bug 207136 is related to this behavior. I am not sure it makes sense to fix this bug 205835 while not addressing bug 207136.
Comment 30 err 2012-01-26 15:30:04 UTC
Has this bug been put into fixes? The related bug 207136 is now in fixes.
Comment 31 Jaroslav Tulach 2012-01-27 07:49:25 UTC
You are right, it seems that the 5a38e871f74e changes are not in release71_fixes branch. Integrated in: d471850f1b22
Comment 32 Marian Mirilovic 2012-02-02 09:03:29 UTC
*** Bug 207973 has been marked as a duplicate of this bug. ***
Comment 33 Tomas Danek 2012-02-15 13:14:26 UTC
works fine now. verified in

Product Version: NetBeans IDE 7.1.1 RC1 (Build 201202132200)
Java: 1.6.0_29; Java HotSpot(TM) 64-Bit Server VM 20.4-b02-402
System: Mac OS X version 10.7.3 running on x86_64; MacRoman; en_US (nb)
User directory: /Users/tomas/.netbeans/7.1.1rc1
Cache directory: /Users/tomas/.netbeans/7.1.1rc1/var/cache
Comment 34 Jiri Prox 2012-02-20 07:23:25 UTC
*** Bug 208598 has been marked as a duplicate of this bug. ***
Comment 35 jvenderb 2012-03-01 09:34:40 UTC
Just updated to 7.1.1. After starting up I get the message that jVI plug-in is disabled because of issue 205835. If I get it right the issue is resolved?
Comment 36 ricktw 2012-03-01 11:02:38 UTC
same here...
Comment 37 err 2012-03-01 16:20:27 UTC
jVi checks module version number (comment 21). This wasn't changed as part of the fix. An updated jVi will be available some time next week.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo