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

RESOLVED FIXED in Firefox 3.6a1

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: dcamp, Assigned: xtc4uall)

Tracking

Trunk
Firefox 3.6a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
Jesse pointed out that "page" might be better wording than "site", since sometimes it's not the entire site that isn't blacklisted.
Created attachment 299006 [details] [diff] [review]
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.
Assignee: nobody → johnath
Status: NEW → ASSIGNED
Attachment #299006 - Flags: review?(dcamp)
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

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

I'd change the entities to phishingPage. and malwarePage....
(Reporter)

Updated

10 years ago
Attachment #299006 - Flags: review?(dcamp) → review+
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.
Attachment #299006 - Flags: approval1.9?

Comment 5

10 years ago
I don't think that the patch should land as is, i.e., without rev'ing the entity names.
Comment on attachment 299006 [details] [diff] [review]
Replace all 'site' to 'page' except one

r-, as per Pike.
Attachment #299006 - Flags: review-
(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 on attachment 299006 [details] [diff] [review]
Replace all 'site' to 'page' except one

Please re-request approval once patch has sufficient reviews.
Attachment #299006 - Flags: approval1.9?
(Assignee)

Comment 9

9 years ago
Created attachment 373008 [details] [diff] [review]
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?
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.
Assignee: johnath → xtc4uall

Comment 11

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

That's bug 488682
(Assignee)

Comment 13

9 years ago
Created attachment 373217 [details] [diff] [review]
patchv2

i hope i got it right though i'm unsure of entities renaming policies.
Attachment #299006 - Attachment is obsolete: true
Attachment #373008 - Attachment is obsolete: true
Attachment #373217 - Flags: review?(dcamp)
(Reporter)

Updated

9 years ago
Attachment #373217 - Flags: review?(dcamp) → review+
(Assignee)

Updated

9 years ago
Attachment #373217 - Flags: review?(l10n)

Comment 14

9 years ago
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.
Attachment #373217 - Flags: review?(l10n) → review+
(Assignee)

Comment 15

9 years ago
Created attachment 375304 [details] [diff] [review]
patch for checkin

(In reply to comment #14)
*done*
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/4995c3f35e8d
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
Created attachment 389692 [details] [diff] [review]
realigning comments

Sorry for the nitpicking, but the comments point to non-existing labels now. Could we have this changed?
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.