139948 - SSL Tooltip not updated when going from one ssl site to another

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Takuo KITAME]

on 1.0RC1 and trunk (2002-04-23).

Go to https://mail.epost.de/
this is a SSL site signed by verisign (check with tooltip of over the lock icon).

Now go to https://www.entrust.net/
have a look at the tooltip. Hey, why do entrust.net have their website
signed by verisign?

Now a more dangerous case: go to
https://ssl.drinsama.de/
and accept the certificate.
check with the tooltip that you still are on a site sigend by
verisign!

Comment 1

[Mitchell Stoltz (not reading bugmail)]

->PSM

Comment 2

[Stephane Saux]

kai.
Can you take a look at this asap?

Comment 3

[Jared Johnson]

It might be of some use to note that Galeon had a bug very similar to this,
which was basically the result of some wise guy who said "Why should we update
the security icon if we're going from one secure site to another??" --
unfortunately, the same function that updated the security icon also updated the
security tooltip.  Someone fix this soon so Debian can apply the patch to 1.0
rc1 before releasing Woody on May 1 :P

Comment 4

[Kai Engert [:KaiE:]]

confirming and accepting bug

Comment 5

[Kai Engert [:KaiE:]]

Jared, I agree, the problem is similar.

I trace the code and see, the security code correctly updates its internal
information to contain the correct new tooltip string.

However, currently security code does not actively push that information to the UI.

Instead, the UI is fetching the string, and seems to cache that string until the
security state changes.

Currently the security code will only notify others when the security state
changes. Obviously this is not correct, it must notify whenever a page finished
loading, and the tooltip text could have potentially changed, to allow the UI to
refetch the tooltip text.

I'm attaching a simple patch, that fixes the problem for me.

Comment 6

[Kai Engert [:KaiE:]]

Attachment: Suggested fix

Comment 7

[Kai Engert [:KaiE:]]

Attachment: Same Patch with bigger context and ignoring whitespace

This patch shows that the change only consists of moving one bracket.
It has the effect that the notifying code will be executed in any case a
document loading stops (no longer only if the state differs from the previous
state).

Comment 8

[Kai Engert [:KaiE:]]

Javi, can you please review?

Jag, can you please superreview?

Comment 9

[Javier Delgadillo]

Comment on attachment 81452 [details] [diff] [review]
Same Patch with bigger context and ignoring whitespace

r=javi

Comment 10

[jag (Peter Annema)]

sr=jag

Comment 11

[André Dahlqvist]

Nominating for mozilla 1.0.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 81452 [details] [diff] [review]
Same Patch with bigger context and ignoring whitespace

a=[e-mail] for branch checkin; please make sure it's in trunk as well

Comment 13

[Kai Engert [:KaiE:]]

Checked in to trunk, fixed.

Comment 14

[scottputterman]

adding adt1.0.0+.  Please check this into the branch and add the fixed1.0.0 keyword.

Comment 15

[Kai Engert [:KaiE:]]

fixed-on-branch

Comment 16

[John Unruh]

Verified on branch.