Created attachment 196605 [details] [review]
network-agent: Focus the first reactive entry

Currently network dialogs don't focus password entries, which means
that rather than entering their password directly, users first have
to click the entry (or tab around the dialog).
Instead, put keyboard focus on the first entry that requires user
input.

Created attachment 196606 [details] [review]
network-agent: Allow entries to activate default action

Currently entries' 'activate' signal is ignored, so hitting enter
does not have any effect, even if all required information has been
entered.
Instead, connect to the 'activate' signal so that hitting enter
behaves as if the "OK" button had been pressed.

*** Bug 658616 has been marked as a duplicate of this bug. ***

Comment on attachment 196605 [details] [review]
network-agent: Focus the first reactive entry

ModalDialog already has code for this; you just need to call this.setInitialKeyFocus()

Comment on attachment 196606 [details] [review]
network-agent: Allow entries to activate default action

this is OK for now (since this is the dialog where everyone seems to notice this problem) but there should probably be functionality at the ModalDialog level eventually

(In reply to comment #5)
> (From update of attachment 196606 [details] [review])
> this is OK for now (since this is the dialog where everyone seems to notice
> this problem)

Yeah, mostly because the polkit dialog already does something similar.


> but there should probably be functionality at the ModalDialog level eventually

Agreed.

Created attachment 196613 [details] [review]
modal-dialog: Don't set initial key focus in _init()

ModalDialog sets the initial key focus to a container which doesn't
set the can-focus property; this doesn't make any sense at all.

Created attachment 196614 [details] [review]
modal-dialog: Don't let buttons steal manually-set focus

ModalDialog provides a method to set the initial focus. However,
when adding buttons, the initial focus is always set to the last
button, thus overwriting a previously set manual focus.
Instead, only set the initial key focus if setInitialKeyFocus()
has not been called manually before.

Created attachment 196615 [details] [review]
polkit-agent: Remove hack to focus password entry

As dialog buttons used to "steal" the initial key focus, the polkit
dialog delayed focusing the password entry. With buttons no longer
overwriting the manually set focus, this is no longer necessary.

Created attachment 196617 [details] [review]
network-agent: Focus the first reactive entry

(In reply to comment #4)
> (From update of attachment 196605 [details] [review])
> ModalDialog already has code for this; you just need to call
> this.setInitialKeyFocus()

Right, but adding the dialog buttons overwrites any previously-set key focus :(

Comment on attachment 196613 [details] [review]
modal-dialog: Don't set initial key focus in _init()

>ModalDialog sets the initial key focus to a container which doesn't
>set the can-focus property; this doesn't make any sense at all.

can_focus really means can_be_navigated_to_with_the_keyboard. Having the focus there means that if there's no other initialKeyFocus, but there are can_focus items in the dialog, then Tab will take you to the first one. Though it's probably a bug if you didn't set initialKeyFocus in that case, so OK I guess

(In reply to comment #11)
> can_focus really means can_be_navigated_to_with_the_keyboard. Having the focus
> there means that if there's no other initialKeyFocus, but there are can_focus
> items in the dialog, then Tab will take you to the first one. Though it's
> probably a bug if you didn't set initialKeyFocus in that case, so OK I guess

Mmmh, but it also means that when there's neither an initialKeyFocus nor buttons, pushModal() won't grab the keyboard at all - not sure it's a corner case we care about, but when modifying attachment 196614 [details] [review] like this:

-            if (!this._initialKeyFocus ||
+            if (this._initialKeyFocus == this._dialogLayout ||
                 this._buttonLayout.contains(this._initialKeyFocus))
                 this._initialKeyFocus = buttonInfo.button;

this patch could be dropped altogether. Sounds like a good idea?

Comment on attachment 196613 [details] [review]
modal-dialog: Don't set initial key focus in _init()

Mark attachment 196613 [details] [review] obsolete as of the last comment.

Attachment 196606 [details] pushed as 92024b7 - network-agent: Allow entries to activate default action
Attachment 196614 [details] pushed as 6d92af1 - modal-dialog: Don't let buttons steal manually-set focus
Attachment 196615 [details] pushed as bc0c490 - polkit-agent: Remove hack to focus password entry
Attachment 196617 [details] pushed as 4e4ce0d - network-agent: Focus the first reactive entry