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

[Chris Hegarty]

Thanks Kurchi, looks fine.

-Chris

Kurchi Hazra <kurchi.subhra.hazra at oracle.com> wrote:

>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/
>
>- Kurchi
>
>>
>> -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/view_bug.do?bug_id=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.
>>>>
>