399234 - Add Exception dialog: make Get Certificate the default button, make buttons accessible

Status: VERIFIED DUPLICATE of bug 399048

(Core Graveyard :: Security: UI, defect)

Description

[Steffen Wilberg]

Attachment: patch

In the Certificate Manager - Servers - Add Exception dialog:
1. The Get Certificate button should be the default.
2. Pressing enter in the text field should get the cert.
3. Tabbing to the Get Certificate or the View button and pressing enter should work.

This patch does that by
1. adding |dlgtype="extra2"| to the Get Certificate button and adding |defaultButton="extra2"| to the dialog.
2. Unfortunately, pressing enter in the textbox does still not work with that, so I added a onkeypress handler.
3. To make the buttons keyboard accessible, I changed the onclick handlers to oncommand handlers.

Comment 1

[Steffen Wilberg]

Requesting blocking because the buttons are inaccessible.

Comment 2

[Kai Engert (:KaiE:)]

Steffen, thanks a lot for your patch. You probably had not seen we tried to work on this in bug 399048, too. There might be conflicts between the patches in both bugs, we should try to merge them.

I see you are proposing a solution to the "default button" problem, which we have not yet solved in the other bug.

I see you are proposing the extra2 button approach. Will this change the position of the button?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> I see you are proposing the extra2 button approach. Will this change the
> position of the button?

It won't. I actually wasn't aware of the code in dialog.xml's _configureButtons that allows you to specify "extra" buttons other than those included in the dialog binding's anonymous content. This patch looks good to me.

Comment 4

[Steffen Wilberg]

I've seen that bug and it looked like you wanted to concentrate on the accesskeys. The merge conflicts are probably trivial.

The button location won't change, it just has that double-border initially for being the default.

Comment 5

[Kai Engert (:KaiE:)]

I just merged your changes over to the other patch and tried it, it works great, thanks!!!

I think we should have a single bug, in order to minimize conflicts.
I'll resolve this one as a duplicate and add your changes to the other one.

Comment 6

[Kai Engert (:KaiE:)]

Comment on attachment 284222 [details] [diff] [review]
patch

removing review request.

I think Johnathan should review it in bug 399048

Comment 7

[Serge Gautherie (:sgautherie)]

V.Duplicate.