1126413 - Firefox should show better indicators for RC4 sites

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Eric Rescorla (:ekr)]

Test case:
https://whatwg.org/

This presently shows a triangle, most likely because it is RC4.
However, this means you also get no site identity information,
which is sad.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Expose nsISSLStatus for broken secure pages

The security UI can not access the information without this change.

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: UI changes

Comment 3

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8556051 [details] [diff] [review]
Expose nsISSLStatus for broken secure pages

Review of attachment 8556051 [details] [diff] [review]:
-----------------------------------------------------------------

I believe this is correct. Be aware that for connections that are broken as in couldn't we even complete the handshake and the user can't add an override, there is no sslStatus object (but that's not what broken represents in this case, since we've connected regardless of some brokenness and do have an sslStatus).

Comment 4

[Justin Dolske [:Dolske]]

Does this regress bug 420246 (which added the nulling)?

I'm a little wary of there being code that's relying on not getting an sslStatus when the connection is broken (ie, isn't otherwise checking if the info it gets is actually valid)?

Comment 5

[Dana Keeler (she/her) [:keeler]]

After applying this patch, I couldn't reproduce anything like the behavior described in bug 420246. I agree the situation regarding the network layer indicating security state isn't optimal, but in the case of a completely broken connection, Necko terminates the connection and shows one of those grey error pages, so I don't think there's too much danger there (unless I'm misunderstanding your concern).

Comment 6

[Masatoshi Kimura [:emk]]

(In reply to Justin Dolske [:Dolske] from comment #4)
> Does this regress bug 420246 (which added the nulling)?
> 
> I'm a little wary of there being code that's relying on not getting an
> sslStatus when the connection is broken (ie, isn't otherwise checking if the
> info it gets is actually valid)?

Mixed content pages already have STATE_IS_BROKEN and already returns SSLInfo because mixed content pages are classified as lis_mixed_security inside nsSecureBrowserUIImpl. So if this change causes any problems like bug 420246, it must have already happened on mixed content pages.

Also I couldn't reproduce such a problem either.

Comment 7

[Justin Dolske [:Dolske]]

Comment on attachment 8556052 [details] [diff] [review]
UI changes

Review of attachment 8556052 [details] [diff] [review]:
-----------------------------------------------------------------

Ok.

Comment 8

[Masatoshi Kimura [:emk]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=847ed5e4e0ce

Comment 9

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6594816e867
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d1bb05dffa7

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f6594816e867
https://hg.mozilla.org/mozilla-central/rev/8d1bb05dffa7