436968 - Expose safe browsing preferences in UI (and turn it on)

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[Sean Murphy]

We will need to expose preferences for safe browsing.

"browser.safebrowsing.enabled" controls the the enabled status for safe browsing as a whole, and also phishing protection.  

"browser.safebrowsing.malware.enabled" will also enable malware checking, but only if the phishing/general protection preference above is enabled.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Sean noted later in bug 358299 that the two prefs were actually independent.

Our current thinking in the channel is that we should just expose a single pref
[ ] Protect me from bad sites
and have it support NSMixedState properly for those who may want to do one or the other.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This needs to block b2.

Are there any other UI requirements (from Google) related to this pref?  Safari has an update/disabled message along with its pref.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Safari pref UI

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Safari pref UI (sb enabled)

Comment 5

[Sean Murphy]

(In reply to comment #2)
> This needs to block b2.
> 
> Are there any other UI requirements (from Google) related to this pref?  Safari
> has an update/disabled message along with its pref.

I think we just have to be certain to always use legal-speak, using vocabulary that indicates we block "suspected" phishing/malware sites (See the last sentence, below).

Google's Requirements:

"If you use the Google Safe Browsing API to warn users about risks from particular webpages, we require that you follow certain guidelines. These guidelines help protect both you and Google from misunderstandings by making clear that the page is not known with 100% certainty to be a phishing site or a distributor of malware, and that the warnings merely identify possible risk.

    * In your end-user visible warning, you may not lead users to believe that the page in question is, without a doubt, a phishing page or a page that distributes malware. When you refer to the page being identified or the potential risks it may pose to users, you must qualify the warning using terms such as: suspected, potentially, possible, likely, may be. "

Safari doesn't use any of those qualifying terms in it's UI, but we might want to remain on Google's good side, since we finally have been allowed to use the data ;).

Comment 6

[Sean Murphy]

Attachment: Patch

Adds safe browsing pref to the Security pane...

Comment 7

[Sean Murphy]

Attachment: Nib

Comment 8

[Sean Murphy]

Attachment: Screenshot of Patch v1

If anyone has any other suggestions or ideas for the actual phrasing of the UI, feel free to offer it :).

Comment 9

[Chris Lawson (gone)]

I think we need the word "possibly" or "potentially" in there somewhere to qualify it (per Google). Not sure that will all fit on one line, though, unless we want to go with an all-encompassing descriptor like "malicious" or "dangerous" for these sort of sites in place of "fraudulent or harmful".

cl

Comment 10

[Stuart Morgan]

Comment on attachment 362908 [details] [diff] [review]
Patch

r=me on the code. I agree that the wording will need some qualification.

Comment 11

[Stuart Morgan]

Comment on attachment 362909 [details]
Nib

Could you give this a behavioral run-through (including the patented Smokey Key Loop Special)?

Comment 12

[Mike Pinkerton (not reading bugmail)]

Why not put this in the "Show a warning when..." section of checkboxes? It seems like this is just another warning like the others.

Comment 13

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 362908 [details] [diff] [review]
Patch

sr=pink

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #12)
> Why not put this in the "Show a warning when..." section of checkboxes? It
> seems like this is just another warning like the others.

It's not just another warning like the others; 

1) it's not a sheet UI-wise, and 

2) we don't know that the conditions for the other two warnings are for certain harmful (e.g., following a link from Bugzilla to http://caminobrowser.org/ or some other site will trigger the https->http warning, and that is not necessarily a dangerous situation), whereas these sites are pretty much assured to be harmful 99.9% of the time.

Also,

3) Safari and Firefox both have their pref separate from (and superior to) the other warning prefs.  (When those two agree on something it's certainly still possible they are both be doing something stupid, but the odds are tilted towards the behavior being good and helpful.)

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 362909 [details]
Nib

3 things:

1) Top margin is 22px; it should be 20px per our guidelines.
2) "Moving from a secure…" is still the first responder; the new pref should be the first responder.
3) Min height of the window is still the old window height; it should be set to whatever the correct current window height ends up being.

r-

Before you respin this, we should determine what we want the text to be (and if we should show the "safebrowsing unavailable" message in the prefs, like Safari, or somewhere that users will actually see it and know they're not protected).

Also, I don't think we should check this in until right before we know we'll be allowed to pull data from the server; we shouldn't expose UI for features that don't work and which we don't know when they might start working, even in nightlies.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

As for the text, I think we could go with something like

Dangerous sites: [X] Warn me when visiting potentially malicious sites
                 Camino can identify and show a warning for sites that may 
                 attempt to steal your personal information or install harmful 
                 software on your computer.  You should avoid visiting sites 
                 that have been identified as potentially dangerous.


This is just a brain dump of my thoughts while making hot chocolate…the hint text obviously needs more work.

If we need more space, I think we can probably move the "potentially"-type stuff so that it's only in the hint text.  The new wording also eliminates one of the two personal pronouns.

We'll also need to produce a localization note for this, so that localizers know they need to keep the "potential"-type qualification language.

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Turn safe browsing prefs back on

Since I don't think we should land the pref UI until we feel the feature UI is shippable, this bug is also the "right" place to flip the prefs back to "true".  This patch does that, when we're ready.

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

If we don't get to it before this bug, we also need to point browser.safebrowsing.warning.infoURL at http://caminobrowser.org/documentation/security/#phishing

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

...or wherever it ends up in bug 451092, but that URL is my current plan.

Comment 20

[Sean Murphy]

Attachment: Patch, for Check-In

Same as the initial patch, but refreshed to apply to the current source.

Ready to be checked in, having passed sr above.

Comment 21

[Sean Murphy]

Attachment: Security.nib (v2)

Nib updated with Smokey's comments, and changed the text to what he mentioned in comment 16, I thought that was excellent.  Thanks for noticing all of those aspects!

The 22 px top margin was because I was measuring with both the label and checkbox selected.  So, all measurements, and the key view loop, has been addressed.

Note: IB 3.x on Leopard used, just in case we need to open and re-save on Tiger.  Being that it's a nib that's loaded on demand, I'm thinking this is not needed though.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 389155 [details]
Security.nib (v2)

r=me, with the caveats that 
1) smorgan approve the text in comment 15

2) we fix the window's min width/height on checkin

Comment 23

[Stuart Morgan]

Comment 16 doesn't seem to meet the 'Notice to Users About Phishing and Malware Protection' requirement; specifically, there's no indication that it may have false negatives (sites that are bad but we don't warn about). In fact "Camino can identify" sort of implies the opposite.

How about:
Camino can try to warn you about sites that may attempt to steal your personal information or install harmful software on your computer.  You should avoid visiting sites that have been identified as potentially dangerous, but be aware that some dangerous sites may not be identified.

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

The first sentence sounds like it has almost-redundant qualifiers in it, and it reads very awkwardly to me.  "Can try" also makes me think that the feature as a whole may not work at all, instead of "it works well, but there are no guarantees that it will catch every single bad site and only those sites".

I'd prefer if we kept false positives ("may attempt") in the first sentence and let the second sentence cover the false negatives ("but be aware
that some dangerous sites may not be identified"), instead of trying to cover both false positives and false negatives in the first sentence (and false negatives again in the second).

Comment 25

[Stuart Morgan]

Works for me.

Comment 26

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Security.nib (v3)

Sean's nib v2 with my nit fixed and the new hint text based on the recent comments:

Camino can warn you about sites that may attempt to steal your personal information or install harmful software on your computer.  You should avoid visiting sites that have been identified as potentially dangerous, but be aware that some dangerous sites may not be identified.

Comment 27

[Stuart Morgan]

Comment on attachment 389342 [details]
Security.nib (v3)

sr=smorgan

Comment 28

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 363741 [details] [diff] [review]
Turn safe browsing prefs back on

Just dotting the 'i's and whatnot.

Comment 29

[Stuart Morgan]

Comment on attachment 363741 [details] [diff] [review]
Turn safe browsing prefs back on

Ship it!

Comment 30

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Shipped!

(on CAMINO_2_0_BRANCH and cvs trunk) Thanks, everyone!