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

Kurchi Hazra kurchi.subhra.hazra at
Tue Oct 16 00:53:56 UTC 2012

Hi Alan,

    Please find an updated webrev with a test included: is the main entry point - it runs 
CheckUserPrefFirst first (which creates and attempts to store a preference),
and then runs CheckUserPrefLater (which attempts to retrieve the 
preference stored by the former).


On 12.10.2012 07:57, Kurchi Subhra Hazra wrote:
> 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:
>>> Bug:
>>> Thanks,
>>> Kurchi
>>> [1]
>> 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.
> - Kurchi


More information about the core-libs-dev mailing list