207456 - tab/shift-tab should cycle through all enabled buttons in account wizard

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2003.05.28 1.4 comm builds, but i believe this is also an issue in
trunk builds --as it was a problem in the 2003.05.23 trunk bits.

i thought this was a dup, but neither nbaca nor i could find an appropriate,
existing bug.

shouldn't the tab key (and shift-tab) cycle through all enabled buttons in a XUL
dialog?

to observe, in the account wizard

1. select Edit > Mail & Newsgroups Account Settings.

2. click the Add Account button to bring up the Account Setup wizard.

3. in the 1st panel (just use the default email account selection), hit tab to
move focus to the Next button, then hit the spacebar to activate it to move to
the next panel.

4. in the Identity panel, note that the Back button at the bottom is now
enabled. hit the tab key twice to move from the Your Name and Email Address
textfields.

expected: focus should move to the Back button.
actual results: focus skips the Back button and is on the Next button.

5. hit shift-tab to try move backwards from Next to Back.

expected: focus moves to the Back button.
actual results: Back button is skipped, and focus is in the Email Address textfield.

i can understand that the Next button in the wizard is the default button --ie,
what's activated when Enter is hit (when focus isn't explicitly moved
somewhere). but tab and shift-tab (iirc) should always move focus in XUL to each
enabled item (only disabled items skipped).

the problem here is that the user is unable to go backwards in the wizard. via
keyboard navigation.

Comment 1

[Samir Gehani]

adt: need info.  Sairuh, is this a regression from Mach V?

Comment 2

[sairuh (rarely reading bugmail)]

yes, this is a regression from ns7.02.

it regressed sometime btwn the 2003.05.09.05 (works) and 2003.05.14.05 (broken)
builds (looking at commercial linux).

Comment 3

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 4

[shliang]

Attachment: patch

Comment 5

[jag (Peter Annema)]

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

It doesn't make sense to me why this patch would fix the bug. Does the focus
code check for hasAttribute("disabled") instead of getAttribute("disabled") ==
"true"?

Comment 6

[shliang]

Attachment: better patch

Comment 7

[jag (Peter Annema)]

Comment on attachment 125428 [details] [diff] [review]
better patch

r+sr=jag

This probably should go on the branch too.

Comment 8

[jag (Peter Annema)]

Attachment: Shuehan's first patch, slightly modified ("true" instead of !val), for 1.4 branch

Comment 9

[shliang]

Comment on attachment 125428 [details] [diff] [review]
better patch

checked in this change on the trunk

Comment 10

[shliang]

resolving

Comment 11

[[e-mail]]

Um... why not just...
this._nextbutton.disabled=this._canAdvance=val;
(although I can't actually duplicate this in 1.4b)

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 125482 [details] [diff] [review]
Shuehan's first patch, slightly modified ("true" instead of !val), for 1.4 branch

moving approval request forward.

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 125482 [details] [diff] [review]
Shuehan's first patch, slightly modified ("true" instead of !val), for 1.4 branch

a=mkaply

Comment 14

[Mike Kaply [:mkaply]]

Please add the fixed1.4.1 keyword when checked in

Comment 15

[Robert Pollak]

According to Bonsai, this approved patch is still not checked in for 1.4.1.

Comment 16

[timeless]

that's correct. 'better patch' was reviewed and committed to the trunk. the much
longer patch which has *no* reviews (but approval) is sitting here in limbo.

i tend to like short patches. neil is on vacation. jag can't review his own patch.

if a driver wants to commit this long patch to the branch then a driver can do
so. i tried to cancel committing when i realized that there weren't any reviews
and i couldn't find the matching patch on the trunk.

Comment 17

[Mike Kaply [:mkaply]]

jag:

which patch should go on the 1.4 branch?

Comment 18

[Mike Kaply [:mkaply]]

Can someone please tell me which fraking patch is supposed to go onto the branch
or none of them are?

Why is the longer patch needed for 1.4?

Comment 19

[Brian Ryner (not reading)]

These patches are fixing the same problem in different ways.  For 1.4, I think 
we should take the one that's been baking on the trunk, which is attachment 
125428 [details] [diff] [review].  Attachment 125354 [details] [diff] would be a "good idea" for the trunk, but is not 
necessary to fix this problem.

Comment 20

[Mike Kaply [:mkaply]]

I checked in the better patch for 1.4. please open a new bug if something else
needs to be checked in to the trunk.