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

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Tony Chang (Google)]

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.

Comment 1

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

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)

Comment 2

[Tony Chang (Google)]

(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.

Comment 3

[Tony Chang (Google)]

Attachment: 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.

Comment 4

[Tony Chang (Google)]

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

Comment 5

[AKIHIRO Misaki (a.k.a Kuden)]

Attachment: screenshot

left -> Web Forgery
right -> web forgery

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

Comment 6

[Alfred Kayser]

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...

Comment 7

[Tony Chang (Google)]

(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.

Comment 8

[Tony Chang (Google)]

(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.

Comment 9

[Tony Chang (Google)]

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

[Alfred Kayser]

That would also work for the themers, as long as there are no changes to /skin/ files required.

Comment 11

[Tony Chang (Google)]

Attachment: show/hide menus in JS and revert css theme changes

Remove changes to css files and handle in JS instead.

Comment 12

[Alfred Kayser]

Thanks!

Comment 13

[Tony Chang (Google)]

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

Comment 14

[Tony Chang (Google)]

Attachment: branch patch

combination of two patches above for branch

Comment 15

[Tony Chang (Google)]

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.