417687 - "Tell me if the site I'm visiting is a suspected forgery" should be reworded to reflect the fact that the page is now blocked entirely

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Jesse Ruderman]

[x] Tell me if the site I'm visiting is a suspected forgery
[x] Tell me if the site I'm visiting is a suspected attack site

should be something like

[x] Block suspected forgery sites
[x] Block suspected attack sites

since we now block the sites entirely (see bug 399233).

k4y pointed out this bug in #granparadiso:
<k4y> i'm not quite sure i understand this "Suspected Attack Site" behaviour stuff
<k4y> this page ... is apparently a suspected attack site and Firefox says "... has been blocked based on your security preferences" except the only option in security preferences are "Tell me if the site I'm visiting is a suspected attack site"

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Not sure it's worth the late-l10n hit, and I'd ship without it in this version. Adding wanted to track it, though.

Comment 2

[AndrewM]

Hardware and OS should be set to All :)

Comment 3

[AndrewM]

(In reply to comment #0)
> [x] Tell me if the site I'm visiting is a suspected forgery
> [x] Tell me if the site I'm visiting is a suspected attack site
> 
> should be something like
> 
> [x] Block suspected forgery sites
> [x] Block suspected attack sites
> ...

To nitpick, I think the first one should probably be:

[x] Block suspected web forgeries

Or perhaps even:

[x] Block suspected web forgeries (phishing attacks)

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

(removing late-l10n and setting TM to hit next version, also assigning to johnath for fun!)

Comment 5

[Johnathan Nightingale [:johnath]]

Now that phishing allows clickthrough (as will malware, given bug 422410) I'm not sure this text needs to change.  We are once again just "telling" users that a site is blocked, but letting them ignore the warning.

Jesse - what do you think about closing this bug since the behaviour is closer to FF2?  Do you still think a change is required?

Comment 6

[Jesse Ruderman]

"Tell me if the site I'm visiting is a suspected forgery" isn't quite accurate, since just telling wouldn't protect against a malicious-site exploit.  On the other hand, "Block suspected web forgeries" isn't accurate either, since it's not quite that heavy-handed.

How about "Warn me if I go to a suspected forgery site"?  The current string might be close enough, though.

Comment 7

[AndrewM]

Also, the title text for the malware and phishing warning pages has changed from 'Suspected [Web Forgery/Attack Site]!' to 'Reported [Web Forgery/Attack Site]!', so perhaps something like "Warn me if I visit a reported [web forgery/attack site]"?

Or maybe I'm getting dangerously close to bike-shedding territory...

Comment 8

[Johnathan Nightingale [:johnath]]

Attachment: Block reported [web forgeries|attack sites]

I think Jesse's quoted conversation in comment 0 is the most persuasive - when someone sees the blocked-page interstitial, the language we use should match the preference we refer to.  Given that we're in no real rush to call attention to the "ignore this warning" link, I don't think we should worry about the "correctness" of saying "Block" vs. warn.

With that in mind, I propose basically Jesse's original text, with the comment 3 nitpick and acknowledging that we prefer to say "reported" in line with the warning text.  That is:

 [ ] Block reported web forgeries
 [ ] Block reported attack sites

Gonna CC Bryan from Thunderbird since they have a similar string here:

http://mxr.mozilla.org/mozilla/source/mail/locales/en-US/chrome/messenger/preferences/privacy.dtd

Comment 9

[Jesse Ruderman]

Comment on attachment 337700 [details] [diff] [review]
Block reported [web forgeries|attack sites]

Looks good to me, but I think you should get code review from a browser peer.  If you had forgotten to rev the string names, I wouldn't have caught that.

Comment 10

[Johnathan Nightingale [:johnath]]

Comment on attachment 337700 [details] [diff] [review]
Block reported [web forgeries|attack sites]

Yeah, no problem, just wanted to make sure it was a phrasing that addressed your concern.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 337700 [details] [diff] [review]
Block reported [web forgeries|attack sites]

>diff --git a/browser/locales/en-US/chrome/browser/preferences/security.dtd b/browser/locales/en-US/chrome/browser/preferences/security.dtd

>+<!ENTITY  tellMaybeForgery2.label     "Block reported web forgeries">
> <!ENTITY  tellMaybeForgery.accesskey "T">

There's no more capital "T" in the string, so this is suboptimal. "B" appears to be free, use that instead?

Comment 12

[Johnathan Nightingale [:johnath]]

Attachment: Good point about the accesskey

Good catch - I did check to make sure the accesskeys were still valid, but apparently neglected capitalization.

Comment 13

[Bartłomiej Brzozowiec (BartZilla)]

(In reply to comment #8)
> (...)
> 
>  [ ] Block reported web forgeries
>  [ ] Block reported attack sites
> 
> (...)

This ASCII-art suggests that these options are disabled by default (which is, unfortunately, not the case). Are you going to disable these options by default in FF 3.1?

Anyway, my question is: what about bug 430741 (about mentioning "Google" somewhere in these options)? Honestly informing users about the entity responsible for blocking pages is _very_ important... Users should know who is a (one and only) data provider for "anti-{phishing,malware}" service (especially that these options are enabled by default and have negative impact on users' privacy (see eg. bug 368255)).

Comment 14

[Johnathan Nightingale [:johnath]]

No, this patch is not changing the default state of the prefs.  It's also not seeking to solve bug 430731.

You mention that Google is the only provider we ship.  It is outside the scope of this bug, but if you know of other providers of phishing & malware data that are interested in providing this information to Firefox users, we'd be interested in talking with them.  In the past, afaik, those calls have gone unanswered but we'd certainly welcome to the possibility.  Addons exist which use other systems, of course, but with nowhere near the network traffic that shipping with Firefox by default would imply.  At any rate, this is scope creep for this bug - if you have a specific service recommendation, I suggest filing a new bug to investigate it further.

Comment 15

[Johnathan Nightingale [:johnath]]

*correction, bug reference in second sentence should, of course, be bug 430741

Comment 16

[Bartłomiej Brzozowiec (BartZilla)]

No, my main point here is that you again failed to mention "Google" somewhere in these options. This is exactly what bug 430741 is about (well, at least my patch there).
So, I want to propose alternative wording: instead of

 Block reported web forgeries
 Block reported attack sites

I think it should be something like

 Block web forgeries as reported by Google
 Block attack sites as reported by Google

Comment 17

[Johnathan Nightingale [:johnath]]

I think the way we handle making it more discoverable is for bug 430741, not this one - it's just a handful of pref changes to switch firefox to use any other provider of the publicly documented safebrowsing protocol, at which point the strings you propose would be incorrect and confusing to someone who had just gone through the process of changing providers, and I definitely don't want to go down the road of dynamic pref-strings and stuff.  There are a lot of ways to increase the visibility of the list provider if we think that's a good idea, but I don't think this bug is an appropriate place for them.  If that bug ends up concluding that we need to change these strings again, so be it.

This bug will make an incremental improvement, by tying the block-page messages more clearly to the prefs, I don't want to sidetrack it on issues already tracked in other bugs just because the same strings are being touched.

Comment 18

[Bartłomiej Brzozowiec (BartZilla)]

(In reply to comment #17)
> I think the way we handle making it more discoverable is for bug 430741, not
> this one - it's just a handful of pref changes to switch firefox to use any
> other provider

Actually, you have already admitted in your comment 14 above that choosing another "safebrowsing" service provider is not likely (huge traffic etc.).

> of the publicly documented safebrowsing protocol,

Sure, it is "publicly documented", however it seems that this is a case of "look, but don't touch".
From http://code.google.com/p/google-safe-browsing/wiki/Protocolv2Spec (client-side part of this protocol is implemented in Firefox 3 (and Google Chrome, BTW)):

"Do not use this protocol without explicit written permission from Google. (...) Copyright 2007 Google Inc. All Rights Reserved. (...) Note: This is not a license to use the defined protocol."

Such restrictions are, of course, totally against the spirit of open/free source software (and free documentation, BTW).
It is also another reason why popping up of another "safebrowsing" provider is not very likely (it would require implementing of server-side of this spec., which is forbidden by Google), so the rest of your explanation doesn't apply, actually...

I still don't understand what is wrong with my proposal. Your justification seems to boil down to: "no, because no".

Comment 19

[Johnathan Nightingale [:johnath]]

You're using language like "you have already admitted" as though I have some hidden agenda here.  I don't, but I do wonder why you think otherwise.  I think the discussion about how best to surface google's role in providing phishing and malware lists is orthogonal to the subject of this bug, which has to do with the mismatch between the current pref language and the current blocking language and behaviour.

This is not "No, because no", this is "Your comments are better placed in the bug to which they are relevant."

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 337876 [details] [diff] [review]
Good point about the accesskey

ui-yarrr = beltzner

Comment 21

[Johnathan Nightingale [:johnath]]

http://hg.mozilla.org/mozilla-central/rev/007e244ce6ec

Comment 22

[Vlado Valastiak [:wladow]]

You should change accesskeys entity names too, so L10N tools can catch them.

Comment 23

[Vlado Valastiak [:wladow]]

Attachment: rename accesskeys

Comment 24

[Johnathan Nightingale [:johnath]]

(In reply to comment #22)
> You should change accesskeys entity names too, so L10N tools can catch them.

I asked online before landing it and was told that revving wasn't necessary since the semantic meaning of the access key (i.e. the action performed) wasn't changing.

Pike, what's official doctrine?

Comment 25

[Johnathan Nightingale [:johnath]]

Vlado - just to be clear - I appreciate you spotting it and providing a patch, I just want to understand the rule here so as to avoid making a mistake in the future

Comment 26

[Vlado Valastiak [:wladow]]

Johnathan: see comment #11 - this is a typical issue you can meet with when changing strings -> the accesskey won't match. If you don't change accesskey's entity name appropriately, L10n tools simply can't catch this and warn you "hey, your accesskey doesn't match the label". That's what is this about.

Comment 27

[:Gavin Sharp [email: [e-mail]]]

Is it really not obvious enough that you need to edit the accesskey when you're editing the string? Do localizers really just edit the strings that don't match en-US in isolation without considering their associated accesskeys?

Comment 28

[Vlado Valastiak [:wladow]]

<Ok, I know this discussion is not related to this bug, but I'll add one more comment.>

It's more obvious when you change the name ;)

Again, there are L10n tools helping us with translations and QA. Many localizers use them instead of editing files in plain-text editors. These tools/scripts rely on the fact that names of label and its accesskey are the same, they differ only in suffix. If you change label's name and you don't touch accesskey's, script suddenly doesn't know what's the accesskey for this label - then it can't warn you about mismatched accesskey or even suggests you the best choise. Yes, you can ever do it manually by finding it in your file, but you don't have to if programmers won't force us to do so.

Another story: you're using a plain-text editor for translating - no automated tools. Once your translation is complete, you wanna check if your accesskeys are chosen appropriately. You can run a script and you see the results in a second. But this script won't catch any accesskey issue on Security prefpane, because programmers have changed a "label-accesskey binding". So the script says "no errors", the localizer is confident with his work and everybody's happy. Then he opens the Security prefpane and what does he find there? Inapropriately chosen accesskey. If he wants to be 100% sure, that there are no such accesskeys, he has to review f.e. 200 files manually. If programmers can help him by changing acceskey's name, it can save a big amount of time for over 60 localization teams currently working on FX localizations. Do you know of a better approach. Let me know, I'll use it.

Gavin, I though you're more familiar with this ;)

I'm a programmer as well as localizer and I can tell you - it's much easier for a programmer to change the name than for a localizer to track those changes.

I know this can be hard to understand for people that have never ever localized such a piece of soft. Therefore you should listen to what the localizers are saying ;)
 

Oh, and sorry, but I have to mention one more thing: 
Pls, don't make our lives harder, try to make them easier. 
(In reply to comment #24)
> since the semantic meaning of the access key (i.e. the action performed) wasn't

A semantic change of a accesskey?? What's that? There's no semantic meaning of accesskey, really. What do you mean by "action performed"? That it does check a checkbox instead of setting the focus in a textbox? That doesn't really matter, localizer doesn't need to know what does the accesskey do.

And just to be sure that there is no confusion (mainly if you're on mac) - an accesskey is not a keyboard shortcut/command key - we do not localize keyboard shortcuts, but we do localize accesskeys. 

More clear now?

Comment 29

[Axel Hecht [:Pike]]

I'm torn on whether accesskeys need to be bumped when changed or not, I'm leaning towards not.

In this particular case, it's moot though. IMHO.

I favour semantic names for strings, and 

-<!ENTITY  tellMaybeAttackSite.label     "Tell me if the site I'm visiting is a suspected attack site">
+<!ENTITY  tellMaybeAttackSite2.label     "Block reported attack sites">

doesn't look right to me.

<!ENTITY  blockReportedAttackSite.label     "Block reported attack sites">

would have been a better name, and then you would have had to change the accesskey, too.

The '2' is a hack to me and, when possible, should be avoided. There are a flock of undocumented heuristics on whether tools ignore it or pad it and if so, where. And I bet different tools do it differently.

Comment 30

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 339469 [details] [diff] [review]
rename accesskeys

Thanks, Axel and Wladow. I should have given it more thought than I did, but it's just sometimes frustrating having to keep in mind all of these l10n idiosyncrasies.

Axel's proposal sounds like the best way forward here.

Comment 31

[Johnathan Nightingale [:johnath]]

Thank you Vlado for the extra detail - I knew most of it, but not all, and I appreciate you taking the time (and spotting this in the first place!)

(In reply to comment #28)
> I'm a programmer as well as localizer and I can tell you - it's much easier for
> a programmer to change the name than for a localizer to track those changes.
> 
> I know this can be hard to understand for people that have never ever localized
> such a piece of soft. Therefore you should listen to what the localizers are
> saying ;)

It's hard to understand, but not because I've never localized software.  :) I've worked in shops that localize to many different languages, and I've worked with localizers directly, but what I've never encountered before, and what lies at the source of my continual confusion, is the notion that string-only changes should require changes to code.  When you talk about it being easier for developers to change the entity than for localizers to spot the change with their current tools, I don't doubt what you're saying, but there is non-trivial code risk in changing an entity purely for the sake of a tool.  Changing the entity means making sure that I find every code consumer of that entity, and that I change all of them, and that I don't typo anywhere.  The consequences of a typo in an entity for a XUL dialog that is rarely visited (and used to work) might go unnoticed for some time after a patch, but render the dialog useless. 

With MXR, at least I have a mechanism for finding all the consumers of the entity, and if I'm careful with my copy-pastes, I can obviously avoid breaking that code, but if I were editing strings somewhere on the platform, I wouldn't even know what other consumers (songbird, tomtom) I was potentially breaking.

As you have pointed out though - this whole conversation is rather outside the scope of this bug, and fundamentally, I appreciate your help, and the work that all our localizers do.  I'll have a new patch coming up shortly which changes all four entites, their usage in the prefs panel, as well as the IDs for the prefs panel checkboxes, which I believe is all instances of the old "tellMe..." IDs.

At some point though, I really believe we need to find tools that get localizers what they need without putting our code quality at risk.

Comment 32

[Johnathan Nightingale [:johnath]]

Attachment: Change IDs

As mentioned, updated entity IDs for all the strings involved

Comment 33

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 339772 [details] [diff] [review]
Change IDs

You need to change the l10n note references on line 4 as well.

Comment 34

[Vlado Valastiak [:wladow]]

Comment on attachment 339772 [details] [diff] [review]
Change IDs

Is this going to land before b1?

Comment 35

[Johnathan Nightingale [:johnath]]

(In reply to comment #34)
> (From update of attachment 339772 [details] [diff] [review])
> Is this going to land before b1?

That's my hope - I've been trying all week, but the tree's been in a sad state.

Comment 36

[Johnathan Nightingale [:johnath]]

Entity changes landed as well.

http://hg.mozilla.org/mozilla-central/rev/1c1f27a84ecd

Comment 37

[Chris Ilias [:cilias]]

user-doc-complete
http://support.mozilla.com/kb/Options+window?bl=n#security_options