Closed
Bug 799009
Opened 12 years ago
Closed 12 years ago
Remove support for obsolete SSL-related warning prompts
Categories
(Core Graveyard :: Security: UI, defect)
Core Graveyard
Security: UI
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla19
People
(Reporter: briansmith, Assigned: briansmith)
References
Details
(Keywords: relnote)
Attachments
(1 file)
(deleted),
patch
|
mayhemer
:
review+
dao
:
review+
|
Details | Diff | Splinter Review |
+++ 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)
Assignee | ||
Updated•12 years ago
|
Summary: Remove support for various → Remove support for obsolete SSL-related warning prompts
Comment 1•12 years ago
|
||
(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.
Assignee | ||
Comment 2•12 years ago
|
||
(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•12 years ago
|
Attachment #669016 -
Flags: ui-review?(dao) → review+
Comment 3•12 years ago
|
||
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+
Assignee | ||
Comment 4•12 years ago
|
||
(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.
Assignee | ||
Comment 5•12 years ago
|
||
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 6•12 years ago
|
||
Backed out due to orange (probably caused by the patch for bug 799007):
https://hg.mozilla.org/integration/mozilla-inbound/rev/c80941a6582b
Assignee | ||
Comment 7•12 years ago
|
||
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•