currently using mozilla build 2002042906 on winNT4, but I have seen this on more recent build also. step to reproduce: 1.load https://digitalid.verisign.com 2.type about:blank/about:cache or about:plugins in the url bar and hit enter Current result: Security state is still set to the prior visited webpage/site. The page info is also wrong, after you click on the lock icon. Expected result: Security state for should be reset for the about: protocol Note: I did a search for 'security' and 'lock icon' but I was couldn't find a dup. This issue might be already covered by a private/protected security bug filed by someone else, but I don't have access to this kind of information.

CC'ing BenB on request (IRC) Note: current result is for tabbed and non tabbed browser mode!

*** Bug 135178 has been marked as a duplicate of this bug. ***

The (closed) dup bug has some code info, for those who can see it (I will not open the other bug per the security policy).

over to PSM

I want to fix it.

The problem is that the current lock icon tracking code does not look for the "about" protocol. It ignores those requests. It tries to use positive identification of request protocols, and only cares for some of them. It ignores the about protocol currently, ignoring that it can be used to load toplevel documents. At first thought I was trying to add detection for that protocol, but I did not find an easy way, and actually I don't like that approach very much. I think, the toplevel document contents define whether the lock icon should be set or not. We should be able to detect any protocol changing that document, whether we know anything about the used protocol or not. I therefore suggest that we process all toplevel document events, regardless of their type. If we are unable to extract security information, we assume the document is not secure. On the other hand, for mixed secure/insecure content detection, we still have to look at the type of protocol. Unfortunately, because there are several document load events (about:layout-dummy-request?) with anything you load, we can't look at everything - because if we did, we always would show a mixed lock icon. So the trick is: Let's look at all top level events, but only look at known protocols for sub-content. If anybody knows a way to distinguish a progress event, that is part of the shown document, from those progress events, that Mozilla needs internally: Please let me know. I would need that information to make the sub-content detection smarter, and not make it dependent on a list of given protocols any more. There is only one thing I don't like about my patch. The current implementation (before my patch) assumes that START and STOP requests seen will always be in balance. With the new patch, now looking for more protocols for top level progress, this is no longer true. At application start, when the first (empty) window gets loaded, we see stop requests, without having seen start requests before :-( Unfortunately as a result I have to ignore stop requests, while we have not seen already start requests. We should test the lock icon code carefully again after having landed this new patch.

Javi, can you please review?

Comment on attachment 83439 [details] [diff] [review] Suggested Fix r=javi

FYI: I get the same result using a chrome: protocol Step to reproduce: 1.load https://digitalid.verisign.com 2.type chrome://communicator/content/tasksOverlay.xul in the url bar and hit enter But I guess your patch will fix this.

HJ, thanks for the info. I just tested. I'm happy that I can confirm the patch fixes chrome URLs, too.

Comment on attachment 83441 [details] [diff] [review] Same patch, ignoring whitespace for easier reviewing sr=rpotts@netscape.com

hey kai, the STATE_START and STATE_STOP notifications should *always* be balanced. The START is fired when the request is added to the loadgroup and STOP is fired when it is removed ! So, they should always balance out. since they are not, it means that someone is doing something *nasty* :-( do you have a way to reproduce this situation? i'd like to take a look and understand what is going on... thanks, -- rick

Rick, it is easy to reproduce. I tested with the branch. - Apply the patch from bug 144056. - set environment variable NSPR_LOG_MODULES to "nsSecureBrowserUI:5" - configure Mozilla to start with a blank page - quit Mozilla, start Mozilla When I do that, I see the debug output SOMETHING STOPS FOR TOPMOST DOCUMENT but I do not see a start notification. I'm looking for OnStateChange notifications, where aWebProgress->GetDOMWindow == the window that we were given during initialization, and aProgressStateFlags & STATE_IS_REQUEST and loadFlags & nsIChannel::LOAD_DOCUMENT_URI and either aProgressStateFlags & STATE_START or aProgressStateFlags & STATE_STOP I only see STATE_STOP. If you want, I'll not check in the patch until you had a chance to look at this.

hey kai, feel free to check the patch in... i'll take a look and see what's going wrong in the notification firing... i'll let you know what i find out. thanks, -- rick

Checked in to trunk.

Woohoo, thanks people :-)

Verified on 5/16 Win2000 trunk.

Comment on attachment 83439 [details] [diff] [review] Suggested Fix carrying over sr= from the whitespace-ignore patch

Comment on attachment 83441 [details] [diff] [review] Same patch, ignoring whitespace for easier reviewing Marking obsolete since review phase is done.

Raising priority, I think this should be ADT 1.

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending Driver's approval. Pls check this in tonight, and add the fixed1.0.1 keyword. Changing to [adt2 RTM].

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.

Checked in to branch.

Verified on branch.