Closed
Bug 1245992
Opened 8 years ago
Closed 8 years ago
Update the Safe Browsing phishing interstitial page
Categories
(Toolkit :: Safe Browsing, defect)
Toolkit
Safe Browsing
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: francois, Assigned: francois)
References
Details
(Keywords: sec-want, Whiteboard: [adv-main47-])
Attachments
(4 files)
The Safe Browsing list has moved beyond just phishing sites (or "web forgeries"). https://googleonlinesecurity.blogspot.ca/2015/11/safe-browsing-protection-from-even-more.html https://googleonlinesecurity.blogspot.ca/2016/02/no-more-deceptive-download-buttons.html We should update our interstitial page to reflect the expanded coverage of that list.
Assignee | ||
Comment 1•8 years ago
|
||
This is the new phishing interstitial in Chrome.
Assignee | ||
Comment 2•8 years ago
|
||
Here's the current interstitial on Desktop: > Reported Web Forgery! > > This web page at itisatrap.org has been reported as a web forgery and has > been blocked based on your security preferences. > > Web forgeries are designed to trick you into revealing personal or financial > information by imitating sources you may trust. > > Entering any information on this web page may result in identity theft or > other fraud. and here's my proposed version: > Deceptive Site! > > This web page at itisatrap.org has been reported as a deceptive site and has > been blocked based on your security preferences. > > Deceptive sites are designed to trick you into doing something dangerous > like installing software or revealing your personal information (for > example, passwords, phone numbers or credit cards). > > Entering any information on this web page may result in identity theft or > other fraud. I would also like to replace the current Fennec wording with the above so that we end up with only one version of this copy as opposed to the current situation where the copy between Fennec and Desktop only vary in style but actually say the same thing. Any comments/suggestions Matej?
Assignee: nobody → francois
Status: NEW → ASSIGNED
Flags: needinfo?(matej)
Comment 3•8 years ago
|
||
Copy looks good, I'd just make a small change to the second paragraph: Deceptive sites are designed to trick you into doing something dangerous, like installing software, or revealing your personal information, like passwords, phone numbers or credit cards.
Flags: needinfo?(matej)
Comment 4•8 years ago
|
||
[Tracking Requested - why for this release]: The long pole on this change is the localization impact: once we settle on the English text we have to let it ride the full train. The sooner we get it in the sooner we can ship, and even if we changed it today we'll be 3 or 4 months behind the actual behavior change in the list. If we land before the March 7 merge day it will ship June 7; if we miss that the earliest is August 2 (8 week cycle).
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35179/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35179/
Attachment #8719977 -
Flags: review?(paolo.mozmail)
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35181/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35181/
Attachment #8719978 -
Flags: review?(paolo.mozmail)
Comment 7•8 years ago
|
||
I'll not be able to get to this review this week, you may want to redirect it if you need it to be ready soon.
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to :Paolo Amadini from comment #7) > I'll not be able to get to this review this week, you may want to redirect > it if you need it to be ready soon. Next week would be fine too. I'm aiming to land it before the next uplift to aurora. If you have someone else in mind though, I'm happy to redirect it.
Assignee | ||
Updated•8 years ago
|
Attachment #8719977 -
Flags: review?(paolo.mozmail) → review?(past)
Assignee | ||
Updated•8 years ago
|
Attachment #8719978 -
Flags: review?(paolo.mozmail) → review?(past)
Assignee | ||
Comment 9•8 years ago
|
||
Panos, is this something you can review? I'd like to make sure it gets into 47.
Updated•8 years ago
|
Attachment #8719977 -
Flags: review?(past) → review+
Comment 10•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past https://reviewboard.mozilla.org/r/35179/#review32831 ::: browser/base/content/blockedSite.xhtml:203 (Diff revision 1) > - <h1 id="errorTitleText_phishing">&safeb.blocked.phishingPage.title;</h1> > + <h1 id="errorTitleText_phishing">&safeb.blocked.phishingPage.title2;</h1> AFAIK common practice is to change the label like this: safeb.blocked.phishingPage2.title I see that there are other cases in this file where the style you chose is used. I'd like to get flod's opinion on this. ::: browser/locales/en-US/chrome/browser/safebrowsing/phishing-afterload-warning-message.dtd:7 (Diff revision 1) > -<!ENTITY safeb.palm.notforgery.label2 "This isn't a web forgery…"> > +<!-- Label of the Help menu item --> I believe using the same format as other localization notes will let l10n tools display the comment properly to localizers: <!-- Localization note (safeb.palm.notdeceptive.label) - Label of the Help menu item --> ::: browser/locales/en-US/chrome/overrides/appstrings.properties:34 (Diff revision 1) > -phishingBlocked=The website at %S has been reported as a web forgery designed to trick users into sharing personal or financial information. > +phishingBlocked=This web page at %S has been reported as a deceptive site and has been blocked based on your security preferences. This string also needs a new label (phishingBlocked2). There are a few instances of this in other files as well.
Comment 11•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past https://reviewboard.mozilla.org/r/35181/#review32841 This looks fine to me for Firefox, but I wonder if these strings are still used in Thunderbird. Perhaps Fallen can confirm.
Attachment #8719978 -
Flags: review?(past) → review+
Comment 12•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past (In reply to Panos Astithas [:past] from comment #10) > ::: browser/base/content/blockedSite.xhtml:203 > (Diff revision 1) > > - <h1 id="errorTitleText_phishing">&safeb.blocked.phishingPage.title;</h1> > > + <h1 id="errorTitleText_phishing">&safeb.blocked.phishingPage.title2;</h1> > > AFAIK common practice is to change the label like this: > safeb.blocked.phishingPage2.title > > I see that there are other cases in this file where the style you chose is > used. I'd like to get flod's opinion on this.
Attachment #8719977 -
Flags: feedback?(francesco.lodolo)
Comment 13•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past (In reply to Panos Astithas [:past] from comment #11) > Comment on attachment 8719978 [details] > MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r?paolo > > https://reviewboard.mozilla.org/r/35181/#review32841 > > This looks fine to me for Firefox, but I wonder if these strings are still > used in Thunderbird. Perhaps Fallen can confirm.
Attachment #8719978 -
Flags: feedback?(philipp)
Comment 14•8 years ago
|
||
https://reviewboard.mozilla.org/r/35179/#review32843 There's no practical different between teststring2.shortDesc and teststring.shortDesc2, the only case where I would ask you to not put the number at the end is for accesskeys/shortcuts (e.g. teststring2.accesskey is a better than teststring.accesskey2). Both Panos' comments are correct, especially the one about renaming phishingBlocked ::: browser/locales/en-US/chrome/browser/safebrowsing/phishing-afterload-warning-message.dtd:7 (Diff revision 1) > -<!ENTITY safeb.palm.notforgery.label2 "This isn't a web forgery…"> > +<!-- Label of the Help menu item --> Yes please, use the format used in the following comments. <!-- Localization note (stringID) - ... --> ::: browser/locales/en-US/chrome/overrides/appstrings.properties:34 (Diff revision 1) > -phishingBlocked=The website at %S has been reported as a web forgery designed to trick users into sharing personal or financial information. > +phishingBlocked=This web page at %S has been reported as a deceptive site and has been blocked based on your security preferences. This definitely needs a new ID.
Comment 15•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past Mostly minor issues, but the string in appstrings.properties needs a new id.
Attachment #8719977 -
Flags: feedback?(francesco.lodolo) → feedback-
Assignee | ||
Comment 16•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35179/diff/1-2/
Attachment #8719977 -
Attachment description: MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r?paolo → MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r?past
Attachment #8719977 -
Flags: feedback-
Assignee | ||
Updated•8 years ago
|
Attachment #8719978 -
Attachment description: MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r?paolo → MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r?past
Attachment #8719978 -
Flags: feedback?(philipp)
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35181/diff/1-2/
Assignee | ||
Updated•8 years ago
|
Attachment #8719978 -
Flags: feedback?(philipp)
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past I've pushed a new revision which should address all of your review comments. It's different enough that I'd prefer if you took another quick look to make sure I didn't do anything stupid. Thanks!
Attachment #8719977 -
Flags: review?(past)
Attachment #8719977 -
Flags: review+
Attachment #8719977 -
Flags: feedback?(francesco.lodolo)
Comment 19•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past https://reviewboard.mozilla.org/r/35179/#review33077 Strings looks good, thanks. (newbie in action using "ship it" on MozReview, let's hope it doesn't steal Panos' review flag...)
Attachment #8719977 -
Flags: review+
Comment 20•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past Urgh, fixing flags. Side question: is there an equivalent bug for Fennec? Can't spot one in the dependency tree of bug 1149867.
Attachment #8719977 -
Flags: review+
Attachment #8719977 -
Flags: feedback?(francesco.lodolo)
Attachment #8719977 -
Flags: feedback+
Updated•8 years ago
|
Attachment #8719977 -
Flags: review?(past) → review+
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #20) > Side question: is there an equivalent bug for Fennec? Can't spot one in the > dependency tree of bug 1149867. This bug actually covers both Desktop and Fennec. I tested both yesterday.
Comment 22•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/34e6eea401e3 https://hg.mozilla.org/integration/mozilla-inbound/rev/6d9614f87daf
Comment 23•8 years ago
|
||
Backed both out in https://hg.mozilla.org/integration/mozilla-inbound/rev/36f8f2d29af6 - there's several browser-chrome tests which apparently depend on that page, https://treeherder.mozilla.org/logviewer.html#?job_id=22445051&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=22445055&repo=mozilla-inbound.
Comment 24•8 years ago
|
||
And https://treeherder.mozilla.org/logviewer.html#?job_id=22456607&repo=mozilla-inbound, I thought there were (at least) three.
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35179/diff/2-3/
Attachment #8719977 -
Attachment description: MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r?past → MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past
Attachment #8719977 -
Flags: feedback+ → review?(francesco.lodolo)
Assignee | ||
Updated•8 years ago
|
Attachment #8719978 -
Attachment description: MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r?past → MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past
Attachment #8719978 -
Flags: feedback?(philipp)
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35181/diff/2-3/
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past Removing these unused entities fixed the (unrelated) tests that broke and caused the backout. The other patch didn't change.
Attachment #8719978 -
Flags: review+ → review?(past)
Comment 28•8 years ago
|
||
Comment on attachment 8719977 [details] MozReview Request: Bug 1245992 - Update the Safe Browsing phishing interstitial page. r=flod,past https://reviewboard.mozilla.org/r/35179/#review33343 Thanks, strings looks good, and now the mobile part makes sense.
Attachment #8719977 -
Flags: review?(francesco.lodolo) → review+
Comment 29•8 years ago
|
||
Comment on attachment 8719978 [details] MozReview Request: Bug 1245992 - Remove unused Safe Browsing strings. r=past https://reviewboard.mozilla.org/r/35181/#review33363
Attachment #8719978 -
Flags: review?(past) → review+
Comment 30•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e157cde861c https://hg.mozilla.org/integration/mozilla-inbound/rev/2b81e1b1dcb3
Comment 31•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4e157cde861c https://hg.mozilla.org/mozilla-central/rev/2b81e1b1dcb3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 32•8 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #31) > https://hg.mozilla.org/mozilla-central/rev/4e157cde861c > +deceptiveBlocked=This web page at %S has been reported as a deceptive site and has been blocked based on your security preferences. Sorry for mentioning this here, spotted during l10n. This line seems to be wrong for all 4 identical and their related occurences. A page and a site are 2 totally different things, so the above is simply incorrect. * A website is a collection of web pages. * A browser is used for viewing web pages. These are definitions all devs should know and keep in mind (and I haven’t made them up). For that reason, viewing an entire website would take weeks if all pages were to be viewed. Furthermore, there were 3 identical strings starting with "The site at %S" in these files. So either I would use > deceptiveBlocked=The site at %S has been reported as a deceptive site and has been blocked based on your security preferences. or > deceptiveBlocked=The web page at %S has been reported as a deceptive page and has been blocked based on your security preferences. (And similar instances of "page" elsewhere.) https://transvision.mozfr.org/?recherche=deceptiveblocked&repo=aurora&sourcelocale=en-US&locale=en-US&search_type=entities Could anyone fix this (maybe even for 47) and keep the definition in mind? Also note the present inconsistent use of "page" and "site" for such error messages: https://transvision.mozfr.org/?recherche=reported+as&repo=aurora&sourcelocale=en-US&locale=en-US&search_type=strings In most cases, these are about pages (hosted on sites allright) that are blocked, not entire sites. Or to put it simple: a page hosting unwanted software or attacking users doesn’t make an entire site bad. Maybe it’s good to review all of them while at it?
Updated•8 years ago
|
Whiteboard: [adv-main47-]
You need to log in
before you can comment on or make changes to this bug.
Description
•