Add a checkbox to the malware interstitial page, for user to opt-in to send m...

Add a checkbox to the malware interstitial page, for user to opt-in to send malware report. The user's choice will be stored as preference in user's profile. 

Landing original change by kewang: http://codereview.chromium.org/5102001/.

BUG=60831
TEST=Relevant unit_tests,browser_tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=70755

[panayiotis, 2011-01-05 19:39:50]

Ke is away for 2 more weeks, trying to land this patch so we can start showing
the checkbox. Ian will flip the right bits in the bug.

[arv (Not doing code reviews), 2011-01-05 21:33:14]

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
File chrome/browser/resources/safe_browsing_malware_block.html (right):

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:118: function
savepreference() {
savePreference

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
Long line

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
Can you use a margin instead of the &nbsp;

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
The label should go around the input

<label><input>Text</label>

otherwise the user cannot click on the label to check the checkbox.

[panayiotis, 2011-01-05 23:00:04]

Thanks. Done, and a few more css tweaks to make it look more like the original
mock by Glenn

current and mock: http://www.dropmocks.com/mOSTN

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
File chrome/browser/resources/safe_browsing_malware_block.html (right):

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:118: function
savepreference() {
On 2011/01/05 21:33:14, arv wrote:
> savePreference

Done.

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
On 2011/01/05 21:33:14, arv wrote:
> Long line

Done.

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
On 2011/01/05 21:33:14, arv wrote:
> Can you use a margin instead of the &nbsp;

Done.

http://codereview.chromium.org/6066011/diff/1/chrome/browser/resources/safe_b...
chrome/browser/resources/safe_browsing_malware_block.html:152: <input
name="checked" id="checkreport" type="checkbox" jsvalues=".checked:boxchecked"
onclick="savepreference()">&nbsp;<label for="checkreport"
i18n-content="confirm_text"></label>
On 2011/01/05 21:33:14, arv wrote:
> The label should go around the input
> 
> <label><input>Text</label>
> 
> otherwise the user cannot click on the label to check the checkbox.

Done.

[lzheng, 2011-01-05 23:28:29]

One nit. I will the UI related to arv.

http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:328: }
I think it will be better to wrap the rest of code in else {}. Otherwise, if
later we want to add more logic to this function, we have to make sure we add
them before the "if".

[panayiotis, 2011-01-05 23:34:22]

done

On 2011/01/05 23:28:29, lzheng wrote:
> One nit. I will the UI related to arv.
> 
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):
> 
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:328: }
> I think it will be better to wrap the rest of code in else {}. Otherwise, if
> later we want to add more logic to this function, we have to make sure we add
> them before the "if".

[arv (Not doing code reviews), 2011-01-06 00:36:06]

LGTM with nit

http://codereview.chromium.org/6066011/diff/18001/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing_malware_block.html (right):

http://codereview.chromium.org/6066011/diff/18001/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing_malware_block.html:166: <form>
This form element is not needed.

[lzheng, 2011-01-06 01:03:58]

LGTM
On 2011/01/05 23:34:22, panayiotis wrote:
> done
> 
> On 2011/01/05 23:28:29, lzheng wrote:
> > One nit. I will the UI related to arv.
> > 
> >
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> > File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):
> > 
> >
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> > chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:328: }
> > I think it will be better to wrap the rest of code in else {}. Otherwise, if
> > later we want to add more logic to this function, we have to make sure we
add
> > them before the "if".

[panayiotis, 2011-01-06 17:09:02]

Thanks. Will wait to get the OK from Ian too.

On 2011/01/06 01:03:58, lzheng wrote:
> LGTM
> On 2011/01/05 23:34:22, panayiotis wrote:
> > done
> > 
> > On 2011/01/05 23:28:29, lzheng wrote:
> > > One nit. I will the UI related to arv.
> > > 
> > >
> >
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> > > File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):
> > > 
> > >
> >
>
http://codereview.chromium.org/6066011/diff/9001/chrome/browser/safe_browsing...
> > > chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:328: }
> > > I think it will be better to wrap the rest of code in else {}. Otherwise,
if
> > > later we want to add more logic to this function, we have to make sure we
> add
> > > them before the "if".