Security Certificate Mismatch Defaults To "OK"

RESOLVED FIXED in mozilla1.8.1beta1

Status

Core Graveyard
Security: UI
--
minor
RESOLVED FIXED
15 years ago
a year ago

People

(Reporter: Lawrence Worth, Assigned: Gavin)

Tracking

({fixed1.8.1})

1.8 Branch
mozilla1.8.1beta1
fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [kerh-coz], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4) Gecko/20030624

While I agree that we need a shut-up-and-don't-ask-me-again option, as suggested
in 209299, the security mismatch warning should default to "Cancel", since it's
to easy to press Enter too many times.


Reproducible: Always

Steps to Reproduce:
1. Visit the URL.
2. See the mismatch warning as described.
3.

Actual Results:  
"OK" was highlighted.

Expected Results:  
"Cancel" should be highlighted (unless I've asked for the window never to come
up again for that site).

Comment 1

15 years ago
Yes, I agree. But it would make bug 209299 even more urgent to fix, since you
can't dismiss the dialog quickly anymore. So they need to be fixed together.
Status: UNCONFIRMED → NEW
Depends on: 209299
Ever confirmed: true
No longer depends on: 209299
-> psm
Assignee: security-bugs → ssaux
Component: Security: General → Client Library
Product: Browser → PSM
QA Contact: carosendahl → bmartin
Version: Trunk → unspecified
(Reporter)

Comment 3

14 years ago
Yes, I agree that this should be fixed in the same
release as 209299, although they don't necessarily
need to be fixed at the same time, so long as they
both appear in the same release.

Comment 4

14 years ago
Mass reassign ssaux bugs to nobody
Assignee: ssaux → nobody

Updated

13 years ago
Component: Security: UI → Security: UI
Product: PSM → Core

Updated

12 years ago
OS: Windows 98 → All

Updated

12 years ago
Whiteboard: [kerh-coz]
Assignee: nobody → gavin.sharp
This will be easier to fix if bug 322155 is fixed.
Depends on: 322155
Hardware: PC → All
Version: Other Branch → Trunk
Created attachment 214906 [details] [diff] [review]
patch

This works, with the patch from bug 322155. Haven't double checked all these changes yet, though.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.8.1
Version: Trunk → 1.8 Branch
Created attachment 216673 [details] [diff] [review]
patch 2

Now that bug 322155 is fixed, this bug is easy to fix. Relies on the changes in bug 284776 (defaultButton attribute).
Attachment #214906 - Attachment is obsolete: true
Attachment #216673 - Flags: review?
Attachment #216673 - Flags: review? → review?(kengert)
The only change in behavior is that the domain mismatch dialog button defaults to Cancel instead of OK. The other parts are just simplification.
Whiteboard: [kerh-coz] → [kerh-coz][patch-r?]

Comment 9

12 years ago
I'm not sure if I'm ok with this change.

In general, I think your change makes sense.
But I wonder what this will mean for consistency. What are the current defaults in our other cert warning dialogs? Do all off them default to the more secure way already, or is it mixed?
(In reply to comment #9)
> I'm not sure if I'm ok with this change.
> 
> What are the current defaults in our other cert warning dialogs? Do all off
> them default to the more secure way already, or is it mixed?

Do you know of any test sites that I can use to test some of the other dialogs? I don't even really know which dialogs I should be looking at. If there are other dialogs that default to "unsafe" actions, then I can fix those too.

Comment 11

12 years ago
Comment on attachment 216673 [details] [diff] [review]
patch 2

I disagree with this patch.

The default behavior for focus in dialogs is platform specific (for example, on Mac OS X the first focusable item should always be focused, see bug 335089).

Also, the focus behavior  needs to be consistent across all our applications, and even in the XUL toolkit. There are good accessibility reasons for this. For example, users with low to no vision are often heavily reliant on the keyboard - and so they should be able to assume (based on platform user interface standards) which button that has focus.

Only in special exceptions should you call focus().

To sum up, the change is not correct on OS X, and is bad on all platforms for the reason of consistent accessibility.

I think the right way to go is simply remove the focus() call - and let the XBL binding do its default thing.

Comment 12

12 years ago
Sorry, a correction:  I agree with the patch, but it should also extend to either remove the explicit focus() call in newserver.js or ifndef XP_MACOSX it.

Comment 13

12 years ago
Gavin, are you working on this bug ?. any updates ?.
(In reply to comment #13)
> Gavin, are you working on this bug ?. any updates ?.

The patch still works, it's just waiting for Kai's review.

To reply to comment 9, all this patch does is remove the focus() calls that interfere with the dialog binding's standard button focusing, so it is making the security dialogs behave as all other dialogs do. It also changes the default button of the domain mismatch dialog to cancel instead of "OK", per this bug's summary, which I believe to be consistent with other "warning" dialogs where there is a "safe" action.

One such example is the dialog you get when you try to navigate to a URL that specifies HTTP authentication information unnecessarily, like http://foo@redhat.com. It defaults to "No".
Attachment #216673 - Flags: ui-review?(beltzner)
Comment on attachment 216673 [details] [diff] [review]
patch 2

Making sure that the default choice is always the safest makes sense to me.
Attachment #216673 - Flags: ui-review?(beltzner) → ui-review+

Comment 16

12 years ago
Comment on attachment 216673 [details] [diff] [review]
patch 2

ok, r=kengert
Attachment #216673 - Flags: review?(kengert) → review+
Whiteboard: [kerh-coz][patch-r?] → [checkin needed][kerh-coz]
Priority: P3 → --
Target Milestone: mozilla1.8.1 → mozilla1.8.1beta1
Checked in on the trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
QA Contact: bmartin → nobody
Resolution: --- → FIXED
Whiteboard: [checkin needed][kerh-coz] → [kerh-coz]
Comment on attachment 216673 [details] [diff] [review]
patch 2

This is a simple fix that I'd like to land on the 1.8 branch, see comment 14 for the patch description.
Attachment #216673 - Flags: approval1.8.1?
Whiteboard: [kerh-coz] → [a181?][kerh-coz]

Updated

12 years ago
Attachment #216673 - Flags: approval1.8.1? → approval1.8.1+
Whiteboard: [a181?][kerh-coz] → [checkin needed (1.8 branch)][kerh-coz]
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)][kerh-coz] → [kerh-coz]

Comment 19

12 years ago
Was comment 12 addressed before check-in?
(In reply to comment #19)
> Was comment 12 addressed before check-in?

No. The patch didn't make a change to that code, it only added a comment. Want to file a new bug about the change you think needs to be made?

Comment 21

12 years ago
bug 342862 filed
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.