799009 - Remove support for obsolete SSL-related warning prompts

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Remove unneeded SSL-related security alerts

+++ This bug was initially created as a clone of Bug #799007 +++

This patch removes the following prompts:

1. Warning, you are about to enter a secure site
2. Warning, you are about to leave a secure site
3. Warning, you are about to submit a form to an insecure site, when you are already on an insecure site.
4. Warning, you are viewing a site with mixed content.

#1 and #2 are just outdated concepts.

For #3: I left in the confirmation about submitting a form to an insecure site from a *secure* site (i.e. mixed form submission). The one I removed was only about submitting a form on an already-insecure site.

For #4, we've already decided to use passive indicators for mixed content, instead of active indicators.

This code is getting in the way of other work that we'd like to do, related to security indicators and other things.

This patch better aligns desktop Firefox with mobile Firefox.

See also bug 799007

https://tbpl.mozilla.org/?tree=Try&rev=0060823b2b55

Comment 1

[Dão Gottwald [:dao]]

(In reply to Brian Smith (:bsmith) from comment #0)
> For #4, we've already decided to use passive indicators for mixed content,
> instead of active indicators.

I'm somewhat worried about this. First, these indicators aren't in place yet. Second, it might make sense to leave this as a fallback for Gecko consumers other than desktop Firefox that might not have these indicators. It's a crappy UI, but maybe better than nothing.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Dão Gottwald [:dao] from comment #1)
> (In reply to Brian Smith (:bsmith) from comment #0)
> > For #4, we've already decided to use passive indicators for mixed content,
> > instead of active indicators.
> 
> I'm somewhat worried about this. First, these indicators aren't in place
> yet.

We already show the mixed content indicator in the address bar: globe vs. lock icon. See https://www.eventbrite.com/ for example. Tanvi's work on bug 782654 will only improve upon that. I posted this patch specifically to help facilitate that work.

> Second, it might make sense to leave this as a fallback for Gecko
> consumers other than desktop Firefox that might not have these indicators.
> It's a crappy UI, but maybe better than nothing.

This prompt is an alert. It has only an "OK" button and it doesn't allow the user to block the mixed content. So, any application that cares about about securing against mixed content problems really needs to implement a better UI anyway.

One of my goals here is to move most of the UI code out of PSM and into UI components. If you think this prompt is important for Firefox, then I am happy to write a patch to move the code into browser/. This is based on Johnathan's suggestion that we do not have any UI code in Core.

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 669016 [details] [diff] [review]
Remove unneeded SSL-related security alerts

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

Looks good.  Thanks, btw!

r=honzab.

::: security/manager/boot/public/nsISecurityUITelemetry.idl
@@ +38,5 @@
>  // WARNING_CONFIRM_<X> + 1 == WARNING_CONFIRM_<X>_CLICK_THROUGH
>  const uint32_t WARNING_CONFIRM_POST_TO_INSECURE_FROM_SECURE = 9;
>  const uint32_t WARNING_CONFIRM_POST_TO_INSECURE_FROM_SECURE_CLICK_THROUGH = 10;
> +//     removed WARNING_CONFIRM_POST_TO_INSECURE_FROM_INSECURE = 11;
> +//     removed WARNING_CONFIRM_POST_TO_INSECURE_FROM_INSECURE_CLICK_THROUGH = 12;

Just a question: why to keep it?

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Honza Bambas (:mayhemer) from comment #3)
> ::: security/manager/boot/public/nsISecurityUITelemetry.idl
> @@ +38,5 @@
> >  // WARNING_CONFIRM_<X> + 1 == WARNING_CONFIRM_<X>_CLICK_THROUGH
> >  const uint32_t WARNING_CONFIRM_POST_TO_INSECURE_FROM_SECURE = 9;
> >  const uint32_t WARNING_CONFIRM_POST_TO_INSECURE_FROM_SECURE_CLICK_THROUGH = 10;
> > +//     removed WARNING_CONFIRM_POST_TO_INSECURE_FROM_INSECURE = 11;
> > +//     removed WARNING_CONFIRM_POST_TO_INSECURE_FROM_INSECURE_CLICK_THROUGH = 12;
> 
> Just a question: why to keep it?

Do you mean, "why did I comment out those, instead of just erasing them?" I commented them out instead of erasing them because I don't want somebody in the future to re-use those values (11 and 12). IMO, this is what we should be doing with all enums in IDLs and elsewhere.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30f8e29f9cd1

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Backed out due to orange (probably caused by the patch for bug 799007):
https://hg.mozilla.org/integration/mozilla-inbound/rev/c80941a6582b

Comment 7

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba4903cc7523

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ba4903cc7523