110195 - master passwords steals focus in prefs

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Erik Neuenschwander]

Setup:
choose the "if it hasn't been used for X minutes" option in master password prefs

Repro:
1) open preferences
2) expand "Privacy & Security"
3) select "Master Passwords"

Expected: category list keeps focus
Actual: the textbox for minutes steals focus

I use keyboard navigation inside prefs, so down-arrow doesn't work for me to
move to SSL sub-category if Master Passwords renders.

Comment 1

[eberry]

Confirmed in W2k and Linux with build 2001-11-14
The OnCheck handler for the radio button probably sets focus to the minutes edit box

Comment 2

[sairuh (rarely reading bugmail)]

erik, does this happen on only that pref panel, or others?

bryner/samir, would this be a more general focus issue, or should this go over
to the psm folx?

Comment 3

[sairuh (rarely reading bugmail)]

really cc'ing bryner...

Comment 4

[Samir Gehani]

-> javi

Looked at the panel initialization code which is setting the value of the text
field and gaining focus as a side-effect.

Comment 5

[sairuh (rarely reading bugmail)]

testing this out... the "If it had not bee used in [n] minutes" radiobutton
needs to be selected in order to see this.

Comment 6

[John Unruh]

cc'ing ssaux

Comment 7

[Stephane Saux]

pref-masterpass.js sets the focus in changePasswordSettings(). This function is
called from the oncommand handler for the radio buttons in pref-masterpass.xul,
but also from the onLoad handler.  It probably would be enough to not set the
focus when changedPasswordSettings() is called from the onload handler.

I'll put it on my plate to test that.

Comment 8

[Stephane Saux]

Attachment: proposed patch

Set focus to the text field only when appropriate.

Comment 9

[Stephane Saux]

kai (or javi) can you review.

Comment 10

[Kai Engert (:KaiE:)]

Comment on attachment 64448 [details] [diff] [review]
proposed patch

Patch looks good to me, just one nit-pick: When you type inside the
passwordTimeout field, the field should already have focus. Therefore, it's not
required the set focus to the field again. I suggest to use parameter
setfocus=false	in the last call in your patch.
r=kaie

Comment 11

[Stephane Saux]

Attachment: updated path

Incorporate changes suggested by Kai.

Comment 12

[Stephane Saux]

Comment on attachment 64618 [details] [diff] [review]
updated path

move has-review from previous patch to this one as per Kai's implicit approval.

Comment 13

[Stephane Saux]

who should super-review this?

Comment 14

[John Unruh]

nsbeta1

Comment 15

[Stephane Saux]

nsbeta1+

Comment 16

[Alec Flett]

Comment on attachment 64618 [details] [diff] [review]
updated path

sr=alecf

Comment 17

[Kai Engert (:KaiE:)]

Stephane, as you produced the patch, do you want to check it in?
Let me know if I should do it.

Comment 18

[Kai Engert (:KaiE:)]

taking

Comment 19

[Kai Engert (:KaiE:)]

Checked in to trunk.

Comment 20

[John Unruh]

Verified on the 5/29 Win2000 trunk.

Comment 21

[Jaime Rodriguez, Jr.]

adt1.0.1- per ADT triage, for the follwoing reasons.

1.  The pref that has this problem isn't the default selection.
2.  My guess is this panel isn't being accessed too often by people not working
on the feature.
3.  It has an easy workaround - you can tab pretty quickly back to the tree of
panels.

Comment 22

[John Unruh]

Verified.