Remove support for obsolete SSL-related warning prompts

RESOLVED FIXED in mozilla19

Status

Core Graveyard
Security: UI
RESOLVED FIXED
5 years ago
11 months ago

People

(Reporter: briansmith, Assigned: briansmith)

Tracking

({relnote})

Trunk
mozilla19
relnote
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 669016 [details] [diff] [review]
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
Attachment #669016 - Flags: ui-review?(dao)
Attachment #669016 - Flags: review?(honzab.moz)
Summary: Remove support for various → Remove support for obsolete SSL-related warning prompts
(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.
(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.

Updated

5 years ago
Attachment #669016 - Flags: ui-review?(dao) → review+
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?
Attachment #669016 - Flags: review?(honzab.moz) → review+
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/30f8e29f9cd1
OS: Windows 7 → All
Hardware: x86_64 → All
Backed out due to orange (probably caused by the patch for bug 799007):
https://hg.mozilla.org/integration/mozilla-inbound/rev/c80941a6582b
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba4903cc7523

Updated

5 years ago
Blocks: 810673
https://hg.mozilla.org/mozilla-central/rev/ba4903cc7523
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Keywords: relnote
Depends on: 810820
Depends on: 844441

Updated

3 years ago
Blocks: 119111

Updated

2 years ago
Depends on: 239946

Updated

2 years ago
Depends on: 564145
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.