If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

"Report Web Forgery..." in the Help menu should be smarter

RESOLVED FIXED

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: Tony Chang (Google), Assigned: Tony Chang (Google))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

11 years ago
We're seeing a relatively large number of what appears to be false positive reports being sent to the report web forgery (false negative) page.  That is, pages that have incorrectly been put on the black list being submitted by more users as a phishing site.  The comments don't seem to agree with this classification.

The guess is that users see the phishing warning bubble, notice that it's a false positive, close the warning bubble, then are unable to find how to report the false positive.  Instead, they find the Help menu entry for "Report Web Forgery..." and enter the URL there.

To try to more accurately get reports put into the right place, we want to make the "Report Web Forgery..." menu item dynamic.  If the user is on a page that has triggered the warning bubble, the menu item would say "Report Incorrect Forgery..." and point to the report_error page.  Otherwise, the menu item would say "Report Web Forgery..." as it currently does.
This is a good idea, and I'd support taking the change on the branch. We should start on the string requirements ASAP, though ... can we just use safeb.palm.notforgery.label? (ref: http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/chrome/browser/safebrowsing/phishing-afterload-warning-message.dtd#15)
(Assignee)

Comment 2

11 years ago
(In reply to comment #1)
> can we just use safeb.palm.notforgery.label?

Yes, I think the "This isn't a web forgery" wording should work just fine.  I'll start work on a trunk patch.
(Assignee)

Comment 3

11 years ago
Created attachment 253563 [details] [diff] [review]
swap help menu item based on context

This adds a second menu item for the "This isn't a web forgery..." and we toggle which one is showing.  To avoid translating a new string, we're borrowing a string from the popup bubble. The downside is that menu entries normally capitalize all words, but this string only has the first word capitalized.
Attachment #253563 - Flags: review?(bryner)
Attachment #253563 - Flags: review?(bryner) → review+
(Assignee)

Comment 4

11 years ago
on trunk

Checking in components/safebrowsing/content/application.js;
/cvsroot/mozilla/browser/components/safebrowsing/content/application.js,v  <--  application.js
new revision: 1.7; previous revision: 1.6
done
Checking in components/safebrowsing/content/report-phishing-overlay.xul;
/cvsroot/mozilla/browser/components/safebrowsing/content/report-phishing-overlay.xul,v  <--  report-phishing-overlay.xul
new revision: 1.4; previous revision: 1.3
done
Checking in components/safebrowsing/content/sb-loader.js;
/cvsroot/mozilla/browser/components/safebrowsing/content/sb-loader.js,v  <--  sb-loader.js
new revision: 1.13; previous revision: 1.12
done
Checking in themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.40; previous revision: 1.39
done
Checking in themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.53; previous revision: 1.52
done
Created attachment 253839 [details]
screenshot

left -> Web Forgery
right -> web forgery

This is OK?
(I am not an English native speaker)
Blocks: 369178

Comment 6

11 years ago
Why making two menu items, when only the label (and command) changes?
It would be much simpler to update the label and command when needed.
Now themers have to include rules to hide one of the two...
(Assignee)

Comment 7

11 years ago
(In reply to comment #5)
> left -> Web Forgery
> right -> web forgery
> 
> This is OK?
> (I am not an English native speaker)

I don't mind using a different string, but if we want this for 2.0.0.x, it sounds like it'll be hard to get a new string.
(Assignee)

Comment 8

11 years ago
(In reply to comment #6)
> Why making two menu items, when only the label (and command) changes?
> It would be much simpler to update the label and command when needed.
> Now themers have to include rules to hide one of the two...

I think this would involve moving the string to a .properties file so we can manipulate at runtime.  But maybe there's some other way to load .dtd strings that I don't know of.  I'm open to suggestions.
(Assignee)

Comment 9

11 years ago
Oh, here's another idea: keep both menu items but toggle the visibility by twiddling the .hidden attribute rather than using CSS.  Would that work better for themers?

Comment 10

11 years ago
That would also work for the themers, as long as there are no changes to /skin/ files required.
(Assignee)

Comment 11

11 years ago
Created attachment 254322 [details] [diff] [review]
show/hide menus in JS and revert css theme changes

Remove changes to css files and handle in JS instead.
Attachment #254322 - Flags: review?(bryner)

Comment 12

11 years ago
Thanks!
Attachment #254322 - Flags: review?(bryner) → review+
(Assignee)

Comment 13

11 years ago
patch "show/hide menus in JS and revert css theme changes" on trunk

Checking in components/safebrowsing/content/sb-loader.js;
/cvsroot/mozilla/browser/components/safebrowsing/content/sb-loader.js,v  <--  sb-loader.js
new revision: 1.14; previous revision: 1.13
done
Checking in themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.42; previous revision: 1.41
done
Checking in themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.55; previous revision: 1.54
done
(Assignee)

Comment 14

11 years ago
Created attachment 254435 [details] [diff] [review]
branch patch

combination of two patches above for branch
Attachment #254435 - Flags: review?(bryner)
Attachment #254435 - Flags: approval1.8.1.3?
Attachment #254435 - Flags: review?(bryner) → review+
(Assignee)

Comment 15

11 years ago
Comment on attachment 254435 [details] [diff] [review]
branch patch

I'm told that further analysis shows that the earlier conjecture is not true.  It's not important to have this for branch.
Attachment #254435 - Flags: approval1.8.1.4?
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.