Port security exceptions front end piece from Thunderbird to SeaMonkey

RESOLVED FIXED in seamonkey2.0a2

Status

SeaMonkey
MailNews: Message Display
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Robert Kaiser, Assigned: Misak Khachatryan)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.0a2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
Bug 429843 did introduce a rough piece mailnews security exception UI to the Thunderbird frontend, we should adopt this on SeaMonkey as well, and follow bug 465795 later for a better implementation.

For now, let's port the simple piece, as mentioned in bug 429843 comment #79 and bug 429843 comment #88.
(Assignee)

Comment 1

9 years ago
Created attachment 350479 [details] [diff] [review]
patch

Seems easy one.
Attachment #350479 - Flags: superreview?(neil)
Attachment #350479 - Flags: review?(neil)
(Assignee)

Comment 2

9 years ago
Created attachment 350480 [details] [diff] [review]
correct patch

oops, i left dump() in code.
Attachment #350479 - Attachment is obsolete: true
Attachment #350480 - Flags: superreview?(neil)
Attachment #350480 - Flags: review?(neil)
Attachment #350479 - Flags: superreview?(neil)
Attachment #350479 - Flags: review?(neil)
(Reporter)

Comment 3

9 years ago
This patch works fine for accessing an account with an invalid cert from the mail window (tested with IMAPs here) but when doing the load from the browser window to update the mailbiff icon and mailnews not open yet, it fails and still displays the "stupid" cert error dialog with no possibility to add exceptions (of course, as the callback is only installed on the actual mail window).

Comment 4

9 years ago
Comment on attachment 350480 [details] [diff] [review]
correct patch

InformUserOfCertError and BadCertHandler (or should it be called nsMsgBadCertHandler?) need to live in mailTasksOverlay.js so that as some point we can hook it up to the biff code Kairo was talking about.

>+    setTimeout(InformUserOfCertError, 0, socketInfo, targetSite);
Nit: unused socketInfo

>+  var params = { exceptionAdded : false };
>+  params.prefetchCert = true;
>+  params.location = targetSite;
Nit: should write this as one big JS object instead of setting the properties separately.

Comment 5

9 years ago
Comment on attachment 350480 [details] [diff] [review]
correct patch

>+    if (!iid.equals(Components.interfaces.nsIBadCertListener2) &&
>+      !iid.equals(Components.interfaces.nsIInterfaceRequestor) &&
>+      !iid.equals(Components.interfaces.nsISupports))
Nit: iid needs to line up on each line.

r+sr=me with all those nits fixed.

I tried to see if I could resolve the bad cert issue on the browser window but unfortunately whenever you have a msgWindow you get alerts for e.g. timeouts.
Attachment #350480 - Flags: superreview?(neil)
Attachment #350480 - Flags: superreview+
Attachment #350480 - Flags: review?(neil)
Attachment #350480 - Flags: review+
(Assignee)

Comment 6

9 years ago
Created attachment 350604 [details] [diff] [review]
nits fixed for checkin

Requesting r,s again, i'm not sure that i understood all right.
Attachment #350480 - Attachment is obsolete: true
Attachment #350604 - Flags: superreview?(neil)
Attachment #350604 - Flags: review?(neil)
Attachment #350604 - Flags: approval-seamonkey2.0a2?

Comment 7

9 years ago
Comment on attachment 350604 [details] [diff] [review]
nits fixed for checkin

>+    setTimeout(InformUserOfCertError, 0, socketInfo, targetSite);
...
>+function InformUserOfCertError(socketInfo, targetSite)
Nit: You never actually use socketInfo, so I don't see the point of passing it.
Attachment #350604 - Flags: superreview?(neil)
Attachment #350604 - Flags: superreview+
Attachment #350604 - Flags: review?(neil)
Attachment #350604 - Flags: review+
(Assignee)

Comment 8

9 years ago
Created attachment 350607 [details] [diff] [review]
for checkin
[Checkin: Comment 9]

Last nit fixed, carrying forward r+, sr+ from Neil.
Attachment #350604 - Attachment is obsolete: true
Attachment #350607 - Flags: approval-seamonkey2.0a2?
Attachment #350604 - Flags: approval-seamonkey2.0a2?
(Reporter)

Updated

9 years ago
Attachment #350607 - Attachment is patch: true
Attachment #350607 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Updated

9 years ago
Attachment #350607 - Flags: approval-seamonkey2.0a2? → approval-seamonkey2.0a2+

Updated

9 years ago
Keywords: checkin-needed
Comment on attachment 350607 [details] [diff] [review]
for checkin
[Checkin: Comment 9]

http://hg.mozilla.org/comm-central/rev/ba6cc9834591
Attachment #350607 - Attachment description: for checkin → for checkin [Checkin: Comment 9]
Assignee: nobody → misak
Keywords: checkin-needed
Target Milestone: --- → seamonkey2.0a2
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Updated

9 years ago
Blocks: 467299
You need to log in before you can comment on or make changes to this bug.