[Freeipa-devel] [PATCH] 147 Set network.http.sendRefererHeader to 2 on browser config

[Endi Sukma Dewata]

Sorry for the late response. I have some questions below.

On 6/11/2012 1:38 PM, Rob Crittenden wrote:
> Petr Vobornik wrote:
>>>>>> This patch adds a functionality which sets Firefox
>>>>>> network.http.sendRefererHeader configuration option to value '2'
>>>>>> which
>>>>>> enables it.
>>>>>>
>>>>>> Possible values:
>>>>>> http://kb.mozillazine.org/Network.http.sendRefererHeader

According to this page the sendRefererHeader option affects all version 
of Firefox and SeaMonkey. However, the patch seems to set the option on 
newer browsers only that don't have navigator.preference() anymore. Is 
this correct? Can the option be set using navigator.preference() on 
older browsers?

>>>>> Should we also add a message when referer is missing to check this
>>>>> setting in about:config?
>>>>
>>>> I'm not sure what you have in mind. We set the referer option so why
>>>> would user check it afterwards?
>>>>
>>>> Yes the ticket was about checking the option but: If user is
>>>> configuring
>>>> the browser he wants the browser configured. So we should set all
>>>> options which are required. This is one of them. We have not been
>>>> notifying the user what was set, so I didn't add such notification for
>>>> this option now as well.
>>>>
>>>> We might want to notify the user what options were changed but it's not
>>>> the topic of this ticket.
>>>
>>> I was thinking more for already configured browsers who then later mess
>>> with this value. It fails in a very non-obvious way.
>>
>> I'm attaching a patch which slightly changes the displayed error message
>> from:
>>
>> Missing or invalid HTTP Referer, missing
>>
>> to:
>>
>> Missing HTTP referer.
>> You have to configure your browser to send HTTP referer header.
>>
>> Also I think we should document how to set it manually. We already have
>> documentation for the rest of browser configuration.
>
> This approach looks ok to me, I think someone with more Javascript
> experience should review the specifics.

The patch tries to replace the ${message} in the message template with 
the actual error message, but the current message template (i.e. 911) 
doesn't contain the ${message}. Is this for future use?

If these are actually OK then ACK on both patches.

-- 
Endi S. Dewata