Closed Bug 316245 Opened 16 years ago Closed 13 years ago

Secure news server icon does not display padlock

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird2.0

People

(Reporter: cilias, Assigned: mkmelin)

References

Details

(Keywords: fixed1.8, regression)

Attachments

(1 file, 2 obsolete files)

Using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051112 Thunderbird/1.5 ID:2005111206

- Create a new profile.
- Create a new newsgroup account for a secure news server. (example: secnews.netscape.com )
- Restart Thunderbird, for SSL settings to take affect.

Result: The icon for that account still does not display the padlock, although the account does seem to access the server with SSL.

Expected result: The padlock should display.

Other info:
- Secure IMAP displays the padlock
- On snews://secnews.netscape.com:563/netscape.mozilla.thunderbird , a Linux user reported this, so I've set the OS to 'All', assuming this is a cross-platform bug. I don't know if it's on Mac.
- the reports on the thunderbird newsgroup are that this appears in Thunderbird 1.5 RC1, so I guess this broke before then. I'll do some back tracking...
http://archive.mozilla.org/pub/thunderbird/nightly/2005-08-17-09-mozilla1.8/
is the first branch build to contain this bug.
I should add that I've been using auto-update for a while, and the icon appears for my main profile, which was created prior to using auto-update.
Based on the regression window, looks like fallout from bug 304466, checked in 2005-08-16 10:39.
(In reply to comment #0)
> - On snews://secnews.netscape.com:563/netscape.mozilla.thunderbird , a Linux
> user reported this, so I've set the OS to 'All', assuming this is a
> cross-platform bug. I don't know if it's on Mac.

snews://secnews.netscape.com:563/dl7d9k$m6j13@ripley.aoltw.net
Melchert Fruitema confirmed this on MacOS X/TB version 1.5 (2005102519).
I see the missing padlock on secure newsservers as well - Linux and Windows.

Verified on a clean install of 1.5 Beta1 and Beta 2 as well as 1.5 RC1 with a new profile, default theme, no userChrome.css options, completely new mail and news setups. The code inside folderPane.css is correct and the image does exist inside the theme.

Tested on secnews.netscape.com
Attached patch proposed fix (obsolete) — Splinter Review
Attachment #203026 - Flags: review?(mscott)
Keywords: regression
Comment on attachment 203026 [details] [diff] [review]
proposed fix

This isn't going to work for non news accounts that aren't secure right? For those, we'll end up calling GetIsSecure when we only want to do that for news.  

I'm still confused why existing news servers show the lock icon but creating the news server in a new profile has this problem.
Attachment #203026 - Flags: review?(mscott) → review-
This could very well be a core bug. Same bug appears in Seamonkey.
Done some backtracking on SeaMonkey. The first SM build (trunk) to have this bug is also the first one with Gecko 1.9a1. 
That build is:
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2005-08-13-06-trunk/

(ie. nightly before it still says 1.8, and the padlock appears in it)
(In reply to comment #9)
> Done some backtracking on SeaMonkey. The first SM build (trunk) to have this
> bug is also the first one with Gecko 1.9a1. 
> That build is:
> http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2005-08-13-06-trunk/
> 
> (ie. nightly before it still says 1.8, and the padlock appears in it)
> 

That looks like the same day I landed Bug 304466. Thanks Chris.
Magnus, I think a better fix would be to take the server object and QI it to an nsINntpIncomingServer. If that QI gives back a valid value, then call server->GetIsSecure(&isSecure); 

so

if (nntpIncomingServer)
  server->GetIsSecure(&isSecure);
else
{
  check the socket server type like we currently do...
}
Attached patch proposed fix, v2 (obsolete) — Splinter Review
Something like this?
Attachment #203026 - Attachment is obsolete: true
Attachment #203541 - Flags: review?(mscott)
Comment on attachment 203541 [details] [diff] [review]
proposed fix, v2

almost perfect I think.

We just need to ditch the 
if (NS_FAILED(rv)) return rv; 

line.

If GetIsSecure failed to return a value, we should still fall through and set the false property on the data source. I can make that change when I check this in though.
Attachment #203541 - Flags: superreview?(bienvenu)
Attachment #203541 - Flags: review?(mscott)
Attachment #203541 - Flags: review+
Comment on attachment 203541 [details] [diff] [review]
proposed fix, v2

sr=bienvenu, w/ that change
Attachment #203541 - Flags: superreview?(bienvenu) → superreview+
for historical purposes in case this ends up on the branch.
Attachment #203541 - Attachment is obsolete: true
fixed on the trunk, leaving open just in case we have to respin thunderbird 1.5. If this tests out well on the trunk we might consider it. (Would be great if others could test a trunk build from tomorrow).
Target Milestone: --- → Thunderbird1.1
I'm not sure that this is the right fix for this (and bug 304466) because setting the socket type isn't getting reflected in the UI. Perhaps setting the socket type also needs to set IsSecure (which would make bug 304466 redundant)?
Both
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051118 Thunderbird/1.6a1 ID:2005111818
and
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051119 SeaMonkey/1.5a ID:2005111909
work for me.

I set up secure NNTP, IMAP, and POP accounts. All displayed the padlock. All are functioning properly.
(In reply to comment #17)
>  because
> setting the socket type isn't getting reflected in the UI. 

Hey Neil, I'm not sure what you mean by this. Are there still more areas where the padlock isn't showing up right?

(In reply to comment #19)
>(In reply to comment #17)
>>because setting the socket type isn't getting reflected in the UI. 
>Hey Neil, I'm not sure what you mean by this. Are there still more areas where
>the padlock isn't showing up right?
No, I was just thinking aloud that an alternative way to address this and bug 304466 is to make POP3/IMAP servers return a correct isSecure attribute. Or you could always switch news servers to the socketType attribute instead, reusing the Never/SSL radiobuttons from the POP3/IMAP server security UI.
Target Milestone: Thunderbird1.1 → Thunderbird2.0
Why is this bug not marked as FIXED?
(In reply to comment #21)
> Why is this bug not marked as FIXED?

Works fine for me to on linux- but it's not fixed on the branch(es) yet. Maybe now would be a good time to get it in there as well? Or maybe it should be fixed another way, like Niel's comment 20. Scott?

Sorry, I see Scott checked it in to the 1_8 and 1_8_0 branches 2005-12-01 14:55 - I see no fixed1.8 keyword though. My linux build of 1.5 says 2005120113, so it missed 1.5 (en-US) by a couple of hours :(. Don't know about windows. Anyway, for current nightly branch builds the patch is working.
Keywords: fixed1.8
I left the bug open in case we re-implement the fix based on Neil's suggestions. 
Target Milestone: Thunderbird2.0 → Thunderbird 3
QA Contact: front-end
Assignee: mscott → nobody
Marking this fixed, will open a new bug to implement comment 20 - make server.isSecure really work for imap/pop. (Have a patch, just need to test some more.)
Assignee: nobody → mkmelin+mozilla
Blocks: 304466
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 3 → Thunderbird2.0
So verified. :-)
Status: RESOLVED → VERIFIED
Created bug 462045.
You need to log in before you can comment on or make changes to this bug.