53977 - Shouldn't be able to access hidden tabs using the keyboard

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect, P3)

Description

[Blake Ross]

Build ID: just pulled

Steps to Reproduce:
(1) Open Form Manager
(2) Click the "Forms Never Previewed" tab
(3) Press the left-arrow ( <- ) button.

You get to a Password Manager tab.  You can also get to the Form Manager when 
in the Password Manager via a similar manuever...

Comment 1

[Blake Ross]

->future, of course

Comment 2

[Stephen P. Morse]

Now that's a real strange one.  I definitely am seeing that behavior.

I'd rather not "future" it because I use that as a polite way to say "won't fix 
ever" without encountering flames from the reporter.  Also futured bugs don't 
get picked up on my bug-list radar.  Instead I want to fix this as soon as we 
start doing checkins for 6.01.  Therefore M19.

For the above reasons, the target milestone field is to be filled in by the 
assignee only, not by the reporter.

Comment 3

[Blake Ross]

well, that is somewhat of a flawed system.  If bugs aren't going to be fixed, 
they should be WONTFIX.  There is no need to clutter the database with open bugs 
that will never be fixed, just as an act of politeness towards the reporter.  
FUTURE suggests the bug will be fixed after rtm, whereas M19 suggests it will 
still be fixed for this release, which clearly isn't the case...

Comment 4

[Stephen P. Morse]

M19 does not mean "fixed for this release."  In fact, nobody knows what the M 
notations mean anymore and so they can be given whatever meaning the assignee 
wishes in order to help him triage his bugs.

It's more than just an act of "politeness" to avoid using wont-fix.  It's a 
means of avoiding wasting time with unnecessary flame wars.

Comment 5

[sairuh (rarely reading bugmail)]

.

Comment 6

[Blake Ross]

Actually, M19 will pretty much coincide with rtm, since M18 will coincide with 
nsbeta3.  Anyways, sure, use the milestones to prioritize and triage however 
you want.

However, I still do not agree with the use of FUTURE as a way to say `i'm not 
fixing this bug, but i don't want to get flamed.'  There are other ways of 
dealing with unhappy reporters (I like to use the ignore method), but polluting 
the database with open bugs that will never be fixed isn't one of them.

Comment 7

[sairuh (rarely reading bugmail)]

spam: mass-moving open password manager (single signon) and form manager
(autofill) bugs to Terri for qa contact. unfortunately, i cannot cc myself with
this form, so feel free and add me if you want to keep me in the loop with any
(but, pls not all :) of these... will also go thru 'em meself, a bit later...

Comment 8

[Blake Ross]

I have a fix.

Comment 9

[Blake Ross]

Attachment: patch

Comment 10

[Blake Ross]

Attachment: patch - fix whitespace also

Comment 11

[Joe Hewitt (gone)]

r=hewitt on the above patch.

Comment 12

[Stephen P. Morse]

Good news.  But in the future please try to remember to move my name to the cc 
list when you remove it from the assigned-to field.  Otherwise I stop getting 
notifications of changes made to the bug report.

Comment 13

[Blake Ross]

alec, can you sr please?

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Looks fine, but how about in the patch to tabBindings.xml:

var next = this.selectedTab[aDir == -1 ? "previousSibling" : "nextSibling"];
if (next && !next.getAttribute("hidden")) {
  this.selectedTab = next;
  next.focus();
}

Comment 15

[Blake Ross]

Attachment: patch w/ suggestion

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

a=[e-mail]

Comment 17

[Peter "jag" Annema]

r=jag on the first bit, not on the s/onclick/onmousedown/.

blake said he'll get a new bug and some discussion on that change.

Comment 18

[Blake Ross]

Fix checked in.

Comment 19

[Terri Preston]

Verified on Win98,W2k Build 2000-11-27-04-Mtrunk
Mac build 2000-11-27-04-Mtrunk 2000-11-27-04-Mtrunk
Linux build 2000-11-27-09-Mtrunk

Comment 20

[Stephen P. Morse]

The patch presented here has a bug in it.  Namely the tab for the list of saved 
passwords no longer appears.  See bug 62509 for details and for the patch to fix 
the bug in this patch.