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 197502

Summary: Code removing graph listener adds it instead
Product: platform Reporter: jirka_x1 <jirka_x1>
Component: GraphAssignee: elotter
Status: RESOLVED FIXED    
Severity: normal CC: spoonerj30, tomwheeler
Priority: P3 Keywords: SIMPLEFIX
Version: 6.x   
Hardware: PC   
OS: Other   
Issue Type: DEFECT Exception Reporter:
Attachments: changing addition of change listener to removal of change listener
New version of the patch, intended to be more verbose with a reference to the bug fixed
Yet another version of the patch, now also containing a unit test that tests for the bug fixed by this patch.

Description jirka_x1 2011-04-06 17:48:56 UTC
org.netbeans.api.visual.graph.layout.GraphLayout.removeGraphLayoutListener(GraphLayoutListener) should remove a listener, but it adds it instead (as apparent from http://hg.netbeans.org/main/file/aad87726c83d/api.visual/src/org/netbeans/api/visual/graph/layout/GraphLayout.java)

IMHO the fix is simple: replace

listeners.add (listener);

with

listeners.remove (listener);
Comment 1 tomwheeler 2011-08-10 02:50:44 UTC
Adding keyword SIMPLEFIX because it's straightforward.  I will suggest it as an issue to fix in the "First Patch" program.
Comment 2 elotter 2011-10-09 16:01:02 UTC
As with Jirka's previous comment, my implementation plan is to make the obvious change from 

   public final void removeGraphLayoutListener (GraphLayoutListener<N,E> listener) {
        synchronized (listeners) {
            listeners.add (listener);
        }
    }

to

public final void removeGraphLayoutListener (GraphLayoutListener<N,E> listener) {
        synchronized (listeners) {
            listeners.remove (listener);
        }
    }

whilst also doing usage searches inside the platform to see whether some other modules depended on this, with potentially erroneous results.
Comment 3 elotter 2011-10-09 16:47:08 UTC
No usages of removeGraphLayoutListener(GraphLayoutListener<N,E> listener) inside platform sources, this should therefore not influence platform or IDE itself.
Comment 4 elotter 2011-10-09 16:53:04 UTC
Created attachment 111733 [details]
changing addition of change listener to removal of change listener
Comment 5 elotter 2011-10-09 16:55:53 UTC
The patch solely influences api.visual (Visual Library API).
Comment 6 elotter 2011-10-09 21:46:18 UTC
Created attachment 111735 [details]
New version of the patch, intended to be more verbose with a reference to the bug fixed

New version of the patch that changes addition of the change listener to removal of the change listener, adding more detailed comments about the delta.
This obsoletes the earlier version of this patch that did not contain any comments.
Comment 7 elotter 2011-10-10 14:25:32 UTC
Created attachment 111787 [details]
Yet another version of the patch, now also containing a unit test that tests for the bug fixed by this patch.

New version of patch, also adding a unit test that should fail if the fix to GraphLayout is not applied.
Comment 8 spoonerj30 2011-10-10 19:19:25 UTC
I've successfully integrated Ernest's patch into my local repository.  I first patched just the test file and ran the tests which failed expectantly. I then patched the class and ran the tests which then passed.

I'm satisfied that his patch will fix the issue and his included test does test the fix. 

Pursuant to sub-section 4 of the "Testing the fix" section in the First Patch Program directives[1] I am now verifying that this patch is OK and ready for inclusion.

[1] http://wiki.netbeans.org/First_Patch
Comment 9 tomwheeler 2011-10-12 01:50:48 UTC
I've just applied the patch and pushed the fix and corresponding test to the NetBeans core-main repository:

   http://hg.netbeans.org/core-main/rev/3d38437c07ee

Ernest, this should be part of a nightly build by Friday (and maybe even Thursday).  After that, please download a nightly build (such as the platform source ZIP file) from the following URL:

   http://bits.netbeans.org/download/trunk/nightly/latest/zip/

Then verify that your patch is included and mark this issue VERIFIED.  I'll then add you to the the credits page, as I did for the other First Patch graduates:

http://hg.netbeans.org/core-main/file/tip/ide.branding/release-toplevel/CREDITS.html

Nice work!  Also, don't forget to add a comment to Geertjan's blog and tell him that this issue is now fixed :-)
Comment 10 elotter 2011-10-12 12:44:38 UTC
Thanks Tom, I will download nightlies as they become available in the next few days - and hope to confirm the correct elimination of this bug.
Comment 11 Quality Engineering 2011-10-14 14:59:50 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/3d38437c07ee
User: Thomas W. Wheeler <tomwheeler@netbeans.org>
Log: Fix for #197502, developed by Ernest Lotter in the First Patch program
Comment 12 elotter 2011-10-14 22:26:36 UTC
Tom: it is confirmed, the nightly build of 201110140600 has this patch integrated.