Closed Bug 653103 Opened 8 years ago Closed 8 years ago

Scam detection generates too many false positives to be useful and should be easier to turn off as interim fix

Categories

(Thunderbird :: Security, enhancement)

enhancement
Not set

Tracking

(blocking-thunderbird5.0 -)

RESOLVED FIXED
Thunderbird 5.0b1
Tracking Status
blocking-thunderbird5.0 --- -

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

(Blocks 1 open bug)

Details

(Keywords: ux-discovery)

Attachments

(1 file, 2 obsolete files)

Spin-off per bug 623198 comment #49, switching the default and providing an easier way to turn the feature off until it is completed are two different issues. For details see the discussion in bug 623198 and bug 320351.
This is bug 623198 attachment 528503 [details] [diff] [review].

Carrying forward ui-review+ from attachment 527159 [details] [diff] [review] of that bug.
See attachment 527160 [details] for a screenshot (slightly different label).
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attachment #528581 - Flags: ui-review+
Attachment #528581 - Flags: review?(mbanner)
(Just for ease of checkin-comment, it was a ui-r=bwinton that rsx11m is carrying forward.)
Keywords: relnote
I'm preemptively nominating this as a blocker for 3.3 so that it stays on the radar towards the approaching string freeze. It's a highly visible issue and a working patch is up for review.
blocking-thunderbird5.0: --- → ?
Keywords: ux-discovery
Whiteboard: [affects l10n][needs review standard8]
I don't know if this needs a test to be acceptable, but also to facilitate an easier review, here the steps to verify that the new link works as intended:

1. Get a message triggering the scam warning, e.g., from
> http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, which should turn gray
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, the link is blue

I won't be able to come up with a Mozmill test (and this would probably depend on bug 652942 being fixed first anyway), but that sequence should be sufficient for a litmus test until the scam feature has been improved.
Flags: in-litmus?(ludovic)
Blocks: mail-scam
I don't think we'd block on this, but I'm reviewing the patch now.
blocking-thunderbird5.0: ? → -
Comment on attachment 528581 [details] [diff] [review]
Merged link version (has l10n impact)

I know Blake has given this ui-r+, but why not just call ReloadMessage() after setting that preference to false? Then the user would have slightly better visible feedback that something has happened, than just having the bar stay there.
The idea was to leave the dialog open, giving the user a chance to still use the "Ignore Warning" button. I'm fine with it either way.
This is an alternative patch based on comment #6 which will now let the scam notification just disappear when the link is pushed. No changes anywhere else.

I'd read Blake's bug 623198 comment #38 as this being in general covered by his ui-review+ approval, otherwise I assume that he will complain asap... ;-)

Step #3 in the steps for testing (comment #4) will have to be changed if this version is used (replace "turn gray" with "disappear).
Attachment #529983 - Flags: ui-review+
Attachment #529983 - Flags: review?(mbanner)
(In reply to comment #8)
> Created attachment 529983 [details] [diff] [review] [review]
> Link hiding the scam notification
> 
> This is an alternative patch based on comment #6 which will now let the scam
> notification just disappear when the link is pushed. No changes anywhere else.

Yeah, I think that's probably the right behaviour.  If the user has said "Don't show me these ever again", I think we can assume they're not that interested in seeing the current one either.

Thanks,
Blake.
Comment on attachment 528581 [details] [diff] [review]
Merged link version (has l10n impact)

Ok, so let's go with the other one.
Attachment #528581 - Attachment is obsolete: true
Attachment #528581 - Flags: review?(mbanner)
Comment on attachment 529983 [details] [diff] [review]
Link hiding the scam notification

> /**
>  *  Set the msg hdr flag to ignore the phishing warning and reload the message.
>+ *  - allow disabling this feature for all messages until lists are in place.
>  */
> function IgnorePhishingWarning()
> {
>   // This property should really be called skipPhishingWarning or something
>   // like that, but it's too late to change that now.
>   // This property is used to supress the phishing bar for the message.
>   setMsgHdrPropertyAndReload("notAPhishMessage", 1);
> }
> 
>+function DisablePhishingWarning()
>+{
>+  Application.prefs.setValue("mail.phishing.detection.enabled", false);
>+  ReloadMessage();
>+}
>+

The addition of the comment seems out of place. I think it'd be better to put the comment on the DisablePhishingWarning function.

r=me with that fixed.
Attachment #529983 - Flags: review?(mbanner) → review+
Whiteboard: [affects l10n][needs review standard8]
Attached patch Final patchSplinter Review
Patch for check-in (ui-r=bwinton, r=standard8).
Attachment #529983 - Attachment is obsolete: true
Attachment #529998 - Flags: ui-review+
Attachment #529998 - Flags: review+
Revised litmus test for Ludo:

1. Get a message triggering the scam warning, e.g., from
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, which should disappear
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, along with the link
Keywords: checkin-needed
Whiteboard: [c-n: comm-central]
Checked in: http://hg.mozilla.org/comm-central/rev/b3a51479167e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: comm-central]
Target Milestone: --- → Thunderbird 3.3a4
Thanks Mark. Step #3 of the revised litmus test is unfortunately now completely ambiguous in comment #13, thus here yet another revision of the test sequence:

1. Get a message triggering the scam warning, e.g., from
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, notification bar disappears
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, along with the link
Whiteboard: [litmus: comment #15]
https://litmus.mozilla.org/show_test.cgi?id=40442
Flags: in-litmus?(ludovic) → in-litmus+
Whiteboard: [litmus: comment #15]
Keywords: relnote
Depends on: 926473
You need to log in before you can comment on or make changes to this bug.