413746 - Use "page" instead of "site" for malware/phishing block pages.

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Dave Camp (:dcamp)]

Jesse pointed out that "page" might be better wording than "site", since sometimes it's not the entire site that isn't blacklisted.

Comment 1

[Johnathan Nightingale [:johnath]]

Attachment: Replace all 'site' to 'page' except one

This sounds fine to me - I think these days that "web site" is the slightly more popular generic term, but it doesn't hurt us to be precise, and we don't want to over-smear.

I left one "site", in the phrase "Web site owners who believe their page has been reported as an attack page in error..." because I think that still makes sense.

Comment 2

[Johnathan Nightingale [:johnath]]

I didn't rev the entities here - I don't know if these terms are even different in other languages. Axel, what do you think?

Comment 3

[Axel Hecht [:Pike]]

I think there is a semantic change here, and that's worth flagging.

I'd change the entities to phishingPage. and malwarePage....

Comment 4

[Johnathan Nightingale [:johnath]]

Comment on attachment 299006 [details] [diff] [review]
Replace all 'site' to 'page' except one

Requesting approval to land this (late-l10n) string change.  It arose out of the malware security review, where people observed that we shouldn't necessarily suggest that whole "sites" are bad, when only certain pages may be.

Comment 5

[Axel Hecht [:Pike]]

I don't think that the patch should land as is, i.e., without rev'ing the entity names.

Comment 6

[Reed Loden [:reed]]

Comment on attachment 299006 [details] [diff] [review]
Replace all 'site' to 'page' except one

r-, as per Pike.

Comment 7

[Johnathan Nightingale [:johnath]]

(In reply to comment #5)
> I don't think that the patch should land as is, i.e., without rev'ing the
> entity names.

Oh shoot, Axel, sorry about that.  I'll attach an updated patch tonight/tomorrow.

Comment 8

[Damon Sicore (:damons)]

Comment on attachment 299006 [details] [diff] [review]
Replace all 'site' to 'page' except one

Please re-request approval once patch has sufficient reviews.

Comment 9

[(mostly gone) XtC4UaLL [:xtc4uall]]

Attachment: patch

(In reply to comment #3)
> I think there is a semantic change here, and that's worth flagging.
> 
> I'd change the entities to phishingPage. and malwarePage....
like this?

Comment 10

[Johnathan Nightingale [:johnath]]

Thanks for reviving this bug, and for the patch!

In actual fact, though, the strings in that file are no longer used! The malware and phishing error pages use this DTD file for their strings:

http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/safebrowsing/phishing-afterload-warning-message.dtd

The properties file was used by the old speech-bubble popup in Firefox 2, but in Firefox 3 and up, we have used the dtd file exclusively.  I should have removed the properties file as part of my checkin for bug 402370 (as you can see in bug 402370 comment 6, I intended to). I've filed bug 488682 to perform that deletion.

Could I convince you to try again with the dtd file?

The other thing to mention is that once you change the entity names in the file, you also need to change the page that references them, so that it points to the right thing. The only page that uses these strings is:

http://mxr.mozilla.org/mozilla-central/source/browser/components/safebrowsing/content/blockedSite.xhtml

That's the actual blocking page.  So down at the bottom, you'll see it referencing those entities, and when the names change they need an update as well.

Comment 11

[Axel Hecht [:Pike]]

Should blockedSite.properties be hg removed then?

Comment 12

[Johnathan Nightingale [:johnath]]

(In reply to comment #11)
> Should blockedSite.properties be hg removed then?

That's bug 488682

Comment 13

[(mostly gone) XtC4UaLL [:xtc4uall]]

Attachment: patchv2

i hope i got it right though i'm unsure of entities renaming policies.

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 373217 [details] [diff] [review]
patchv2

r=me with nit, I'd prefer to make the label for the button safeb.palm.reportPage.label, similar to the key changes you did elsewhere. The trailing 2 is a hack that we should try to avoid.

Comment 15

[(mostly gone) XtC4UaLL [:xtc4uall]]

Attachment: patch for checkin

(In reply to comment #14)
*done*

Comment 16

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/4995c3f35e8d

Comment 17

[Kadir Topal [:atopal]]

Attachment: realigning comments

Sorry for the nitpicking, but the comments point to non-existing labels now. Could we have this changed?