Phishing toolbar needs to be updated to use new nsIUrlListManager interface

RESOLVED FIXED in Thunderbird 20.0

Status

Thunderbird
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Magnus Melin, Assigned: mconley)

Tracking

Trunk
Thunderbird 20.0

Thunderbird Tracking Flags

(thunderbird19 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Seen starting https://hg.mozilla.org/comm-central/rev/6996e05c74ee but that hardly caused it. Some core change perhaps?

SUMMARY-UNEXPECTED-FAIL | c:\talos-slave\test\build\mozmill\message-header\test-phishing-bar.js | test-phishing-bar.js::test_phishing_bar_for_eml_attachment
  EXCEPTION: Notification bar is collapsed!
    at: test-folder-display-helpers.js line 2831
       assert_true test-folder-display-helpers.js 2831
       assert_phishing_bar_visible test-phishing-bar.js 42
       test_phishing_bar_for_eml_attachment test-phishing-bar.js 89
       Runner.prototype.wrapper frame.js 582
       Runner.prototype._runTestModule frame.js 652
       Runner.prototype.runTestModule frame.js 698
       Runner.prototype.runTestDirectory frame.js 522
       runTestDirectory frame.js 704
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183
(Assignee)

Comment 1

5 years ago
I'm bisecting this one today.
Assignee: nobody → mconley
(Assignee)

Comment 2

5 years ago
Bisecting this has revealed it to be fallout from bug 811193.
Blocks: 811193
(Assignee)

Comment 3

5 years ago
So, if I'm reading this correctly - this didn't just break a test; this has actually busted our ability to detect phishing emails. Nice.

Looks like just an interface change, anyhow.
(Assignee)

Comment 4

5 years ago
I'm not entirely sure what needs to happen here to fix our phishing detection. Clearly, because of https://bugzilla.mozilla.org/attachment.cgi?id=681049&action=diff , we need to change what we're passing to safeLookup.

Any idea what we're supposed to do to change the key from an ACString to an nsIPrincipal?
Bug 775796 may provide some hints
(Assignee)

Comment 6

5 years ago
Created attachment 684284 [details] [diff] [review]
Patch v1

This fixes the test for me, and causes the phishing toolbar to appear.

Try builds coming here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=1ec926fe3faa
(Assignee)

Updated

5 years ago
Summary: Permanent orange: TEST-UNEXPECTED-FAIL | test-phishing-bar.js::test_phishing_bar_for_eml_attachment → Phishing toolbar needs to be updated to use new nsIUrlListManager interface
(Assignee)

Updated

5 years ago
Attachment #684284 - Flags: review?(mbanner)
(Reporter)

Comment 7

5 years ago
Comment on attachment 684284 [details] [diff] [review]
Patch v1

Review of attachment 684284 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/phishing/list-warden.js
@@ +164,5 @@
>    this.debugZone = "multitablequerier";
> +
> +  let uri = Services.io.newURI(url, null, null);
> +  this.principal_ = Components.classes["@mozilla.org/scriptsecuritymanager;1"]
> +                              .getService(Components.interfaces

Services.scriptSecurityManager
Comment on attachment 684284 [details] [diff] [review]
Patch v1

Review of attachment 684284 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me
Attachment #684284 - Flags: review?(mbanner) → review+
(Assignee)

Comment 9

5 years ago
(In reply to Magnus Melin from comment #7)
> Comment on attachment 684284 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 684284 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/components/phishing/list-warden.js
> @@ +164,5 @@
> >    this.debugZone = "multitablequerier";
> > +
> > +  let uri = Services.io.newURI(url, null, null);
> > +  this.principal_ = Components.classes["@mozilla.org/scriptsecuritymanager;1"]
> > +                              .getService(Components.interfaces
> 
> Services.scriptSecurityManager

Ah, way better. Wasn't aware that the script security manager was in there. Thanks Magnus!
(Assignee)

Comment 10

5 years ago
Created attachment 684413 [details] [diff] [review]
Patch v2 (r+'d by Standard8)

Fixed patch
Attachment #684284 - Attachment is obsolete: true
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/comm-central/rev/f402639db541
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
(Reporter)

Updated

5 years ago
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 12

5 years ago
Comment on attachment 684413 [details] [diff] [review]
Patch v2 (r+'d by Standard8)

Looks like bug 811193 got landed on mozilla-aurora, so that's busted us there.
Attachment #684413 - Flags: approval-comm-aurora?
Attachment #684413 - Flags: approval-comm-aurora? → approval-comm-aurora+
http://hg.mozilla.org/releases/comm-aurora/rev/c59a69b8db63
status-thunderbird19: --- → fixed
You need to log in before you can comment on or make changes to this bug.