Start by opening a standard page loaded over http. Then go to a https address. Actual behaviour: ================= While the https page is being loaded, the lock icon will stay in its state until the page has been fully loaded. Go to https://www.kuix.de/misc/test16/nothing.html to see the problem. That page uses a ten second delay before the contents will be fully delivered. The lock icon stays in the unsecure state during the delay. Expected behaviour: =================== The lock icon should indicate the known security state as early as possible. In the above test case, the lock icon should indicate the secure state before the 10 second timeout is over. It was argued in the past, what we are currently doing is ok, because we should not show the new state until it is completely known. I think we should go away from that approach. I think it is reasonable to show the new security state as soon as data is being transfered for a new page. This new mode does not need to be perfect yet, it is ok if we show the best we yet know. While the page continues being loaded, we will continue to check what the state of the loaded page is. If we detect a change, we can just change the UI again.

This patch seems to work for me.

Javi, can you please review?

We need to bake this on the trunk. I mark it adt1 RTM because any lock icon issue is guranteed to touch everybody using this browser. The patch was discussed with evangelism, and at least one large bank has raised the issue.

What happens when an SSL site has content that's both SSL and non-SSL? It sounds like the icon would show "locked", then flip to "unlocked" once non-SSL content was parsed.

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

Re: comment #4. That behavior sounds reasonable to me. It seems reasonable to start by assuming that an https web page is/will-be secure, and so show the lock as locked when we start loading the page. And then if we find, while loading the page, that there are insecure components, unlock the lock. Communicator behaves much the same way as N6 does with respect to this issue. There have been many complaints about this behavior over the years. In this case, I think that "that's how Communicator did it" is not a very good reaason for N6 to have the current lock behavior. Of course, any change to the lock behavior is risky. If someone undertakes to make this change, the result must be thoroughly tested to ensure that we don't create new situations in which the lock state is wrong.

shadow: What you assume is correct.

I still don't mind making this change on the trunk. Given that the current behavior is the same as that on N4, and that changing this behavior is inherently risky, I don't know if it's a good idea to put it on the branch. If the behavior is validated on the trunk, and there's a strong push from evangelism folks to include this change, then we will try and get it on the branch.

Still need sr=. I agree with Stephane's comments about it being pretty late to make a change like this. I'd rather get beat up about showing the proper state a little late than take chances at being beat up about showing the *wrong* state (which would be a bona-fide pull-it-off-the-wire type of bug and could slip the schedule.) My recommendation would be to get this on the trunk and baking and then release it in Buffy.

Rick, can you please review?

Please remember: The lock icon does tell the user what happened in the past. If everything received in the past, while loading the current page, was secure, why not show that information? The lock icon does not make any statement whether the future will be secure. It only says, whether the data shown on the page is secure or not. Loading a page can take a long time to complete. There are pages, that contain some slow content, which can cause the user to look at a secure page, and we display an insecure lock icon - and that is incorrect, too. Also remember users on slow modems. Please note there is not only "going from insecure to secure", but the much worse "going from insecure to secure" scenario: Imagine a user is looking at a secure web page. Next the user clicks on an insecure link, and the insecure page begins to load. If there is a delay in loading the insecure page, because of a stale image, we are currently still showing the secure lock icon! The user might continue to assume the shown page is secure, although it is not! I think that's a good argument for updating the lock icon as early as possible. I have prepared a demonstration. Go to: https://www.kuix.de/misc/test18/ Accept the server certificate. The lock icon will switch to secure. Then lick the link. The next page will load from an insecure http connetion, the word "hello" you are reading on screen will have been received insecurely. This test page has sub content which will not load before a 60 second delay. During that time, the lock icon will still indicate a secure page. With the patch attached to this bug, the lock icon will immediately switch to the insecure mode. It was said, it is a bad thing to show the user a "bad security" icon, even though everything on the current page has been received securely. It was said, "it's better to wait to be sure". But why wait when there has been nothing insecurely received yet? Today's web content is mostly complex, and it is easy to come into a situation, where one image loading from an advertisement server is stale. Imagine a banking site where this happens. The user would always think the page is "not secure", although nothing insecurely has been transfered. In addition, as soon as something insecure will be received, we will switch the lock icon, and if the user did not disable the warnings, he will even get another warning dialog box, informing about the changed situation.

I think this is a regression. In previous versions of Mozilla (like 0.9.4 based, N6.2.3), the lock icon did show incorrect states. However, it did switch the lock icon at an early time. The test case I'm listing in the previous section causes the lock icon to switch very early in that old version of Mozilla. What we see now is a result of the new lock icon tracking implementation, as it was done with the patch in bug 130949.

To minimize risk in this patch, I'm no longer introducing a new point of time to obtain the security state from the channel. The basic change is: We set our internal security state more often. - after the top level document has been loaded, the security state of that document will be checked, and calls the already existent code to update the security UI - we no longer wait for all subdocuments to load completely, but every time a subdocument has been loaded (or loading has been canceled), we check the state for changes again.

Comment on attachment 87214 [details] [diff] [review] More conservative patch, v2 r=javi

Comment on attachment 87214 [details] [diff] [review] More conservative patch, v2 sr=rpotts@netscape.com

Let's get this on the trunk. junruh, can you verify this after it lands?

Checked in to trunk, marking fixed. Finally, my server is online again, so can now again actually access the prepared testcases. When verifying, you can compare the behaviour of the 4 testcases at https://www.kuix.de/misc/test19/ with previous and new builds.

Verified on 6/17 Win2000 trunk. The lock icon behavior looks good to me.

Is thi schange in the branch? I want to make sure that this gest into the branch for the next commercial release. This is such an important change that is being requsted by several banks in Japan. Thanks.

Marking adt1.0.1+ on behalf of the adt for checkin to the 1.0 branch. Please get drivers approval before checking in.

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 1_0 branch.

junruh - can you pls verify this fix on the 1.0 branch? thanks!

Verified on 6/20 branch.

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

This patch caused regresion bug 154084, which makes navigation on some secure sites completely unbearable.

Indeed. Please see http://bugzilla.mozilla.org/show_bug.cgi?id=154084#c9 for the explanation and the same bug for the fix.

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