Code Review Request: 7198073: (prefs) user prefs not saved [macosx] (original) (raw)

Kurchi Subhra Hazra kurchi.subhra.hazra at oracle.com
Fri Oct 12 14:57:59 UTC 2012


On 10/12/12 12:43 AM, Alan Bateman wrote:

On 12/10/2012 01:21, Kurchi Hazra wrote:

Hi,

This is a regression introduced by the fix for 7160252[1] that I pushed a few weeks ago. We should really be using the class member field isUser, rather than the argument passed in the constructor as a parameter for cfFileForNode(). Due to this wrong parameter being passed, user preferences were never getting stored into permanent storage. Webrev: http://cr.openjdk.java.net/~khazra/7198073/webrev.00/ Bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7198073 Thanks, Kurchi [1] http://cr.openjdk.java.net/~khazra/7160252/webrev.02/ Kurchi - thanks for tracking this one down, looking at it now it is obvious how this slipped through. I think the proposed change is okay although I think we need to do a bit of clean-up on this code at some point (not for this change, I realize this one needs to be fixed in 7u and I don't want to muddy the waters). Right, I will have to spend more time cleaning up this after committing this fix.

The other thin is tests. The preferences implementation that came via the Mac port is turning out to have a bit of a bug tail and one or two regressions have sneaked in along the way too. Do you think you could add a test for this issue? Let me get back to you on this.

Also at some point we need to look at the test coverage and quality of the tests for this area as most/all of these Mac specific issues should have been caught before it ever went in. I agree. Again, this will be a longer time goal for me.



More information about the core-libs-dev mailing list