Code Review Request: 7160252: (prefs) NodeAddedEvent was not delivered when new node add when new Node (original) (raw)

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Wed Jul 11 23:45:04 UTC 2012


On 7/11/12 4:24 PM, Chris Hegarty wrote:

On 12 Jul 2012, at 00:15, Kurchi Hazra<kurchi.subhra.hazra at oracle.com> wrote:

Thanks for the review Alan. Updated webrev: http://cr.openjdk.java.net/~khazra/7160252/webrev.01/ Looks fine. Trivially, is there an opportunity to make any fields final since initFields is replaced with a constructor?

Thanks for pointing that out. How about: http://cr.openjdk.java.net/~khazra/7160252/webrev.02/

-Chris.

- Kurchi

On 7/11/12 5:45 AM, Alan Bateman wrote: On 26/06/2012 22:57, Kurchi Hazra wrote: Hi,

On Mac OS X, for Preferences, a new child added event was not being delivered to a NodeChangeListener since the existing code depended on the return value of addNode() in the native code, which returns true if a new node is added. However, since addNode() was being called erroneously after a child node is already added to an existing node, addNode() would always return false, resulting in thw new node event never being delivered. This fix propagates the required information of whether a node is added from the method adding the child node itself. In addition, I cleaned up the constructors in MacOSXPreferences.java and added a test (AddNodeChangeListener.java) to cover this case. Finally, there were two prefs tests in ProblemList.txt which are now passing, I have removed these from the ProblemList too.

Bug: http://bugs.sun.com/viewbug.do?bugid=7160252 Webrev: http://cr.openjdk.java.net/~khazra/7160252/webrev.00/ Thanks, Kurchi It doesn't look you got a reviewer for this one. I looked at the changes and the fix seems okay to me. The only odd thing is that now that you have the 5-arg constructor then it can initialize all the fields allowing you to get rid of initFields and also setNew. Also I assume it means you can close 7150557. -Alan.



More information about the core-libs-dev mailing list