Closed Bug 461052 Opened 16 years ago Closed 15 years ago

messagereader's delete and junk buttons shouldn't keep focus after use

Categories

(Thunderbird :: Message Reader UI, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: philor, Assigned: Bienvenu)

References

Details

(Keywords: dataloss, Whiteboard: [no l10n impact])

Attachments

(1 file, 2 obsolete files)

STR:
1. Select the first of three messages
2. Click the trash can button in the envelope pane to delete it
3. When the second message loads in the preview pane, press the spacebar to scroll it if it's longer than one page, or to jump to the third message if the second is short.
4. Notice that you now only have one message, because the trash can button retained keyboard focus, so your space-to-scroll was actually a space-to-activate.

(Note to anyone who doesn't get action on something you asked me to do in a bug over the last three or four days: I may have deleted the bugspam instead of keeping it to remind me, since this isn't as obvious if you select the 1501st of 1800 messages in a folder.)
Flags: blocking-thunderbird3?
Oh, and the junk button too - that would explain why a todo reminder I sent myself was in the junk (luckily, sometime recently I stopped sending stuff I mark directly to the trash).
Summary: messagereader's delete button shouldn't keep focus after use → messagereader's delete and junk buttons shouldn't keep focus after use
This is hitting me in my WIP archive button as well.
Flags: blocking-thunderbird3? → blocking-thunderbird3+
a target milestone was never set for this - setting to b2.
Target Milestone: --- → Thunderbird 3.0b2
giving to Dan since he's been most involved with message reader.
Assignee: nobody → dmose
Whiteboard: [no l10n impact]
I'm wondering if we could transfer focus from the buttons to the message pane after the button has been clicked.  I might try a quick patch to see if I can make this work, but the general approach seems like the right thing.
Status: NEW → ASSIGNED
Component: Mail Window Front End → Message Reader UI
QA Contact: front-end → message-reader
Whiteboard: [no l10n impact] → [no l10n impact][target slushy]
After some discussion, Bryan, David, and I came to the conclusion that it's probably better to transfer focus to the threadpane after one of the buttons-that-belongs-to-the-message has been clicked, so I'm going to start on a patch to do that.

Marco, do you anticipate this being an accessibility problem?  If so, what would be preferable for accessibility and why?
(In reply to comment #6)
> Marco, do you anticipate this being an accessibility problem?  If so, what
> would be preferable for accessibility and why?

No, I don't expect this to be an accessibility issue esp for keyboard only users. For one, someone like me who uses the keyboard exclusively will probably not hit that button using the mouse, and thus never run into that problem. Second, keeping the focus on the "Delete" button is, indeed, quite dangerous, so I'd always prefer that such an action should never be executable automatically.

One question: Why did you choose the thread pane instead of the body area of the then newly loaded message?
The idea of choosing the thread pane is that keyboard nav there (e.g. when navigating up/down the list) still works.  keyboard nav in the message pane is deemed less useful, but navigating the threadpane w/ the keyboard and then using the mouse for clicking on those messagepane buttons _feels_ like a common scenario.
As a keyboard user you could hypothetically still hit it, by tabbing into the header pane and over to the archive or junk buttons and hitting space to activate, instead of using the keyboard shortcut, then hitting space thinking the new message will scroll. (You as a screen reader user who always knows where focus is, however, would not, and I assume we're not going to screw with your current flow of having focus in the message body, hitting N or A, and still having focus in the message body, but are only going to make the buttons do focus-shifting.)
My understanding from talking to davida is that he's been discussing this with bienvenu, and bienvenu has expressed some interest in running with this.  Reassigning.
Assignee: dmose → bienvenu
I talked to davida about clearing the message window before displaying a new message; I think that's somewhat orthogonal to this issue. It's loosely related in the sense that we want to know what message an operation will apply to, I guess, but I hadn't planned on tackling this. Who did this bug come from? I can try transferring focus to the thread pane, I guess, but I'd need to figure out the control flow here...
Attached patch wip (obsolete) — Splinter Review
this would work, except that it doesn't deal with the stand-alone message window. I need to think about that...
Comment on attachment 362630 [details] [diff] [review]
wip

and the stand-alone message window has this same bug, and worse, because it treats space as pressing the focused button, and the normal meaning of "advance"
Attached patch proposed fix (obsolete) — Splinter Review
handle 3-pane and stand-alone message window.
Attachment #362630 - Attachment is obsolete: true
Attachment #362637 - Flags: review?(philringnalda)
Whiteboard: [no l10n impact][target slushy] → [no l10n impact][has patch, needs review philor]
philor point out over IRC that I missed the reply button...
Attachment #362637 - Attachment is obsolete: true
Attachment #362641 - Flags: review?(philringnalda)
Attachment #362637 - Flags: review?(philringnalda)
Comment on attachment 362641 [details] [diff] [review]
do the same for reply button

That works fine for me, though someday soon we're going to have to deal with all these buttons (the header buttons, notification bar buttons, dunno what else) that both do their action and also scroll the content when you activate them with a space.
Attachment #362641 - Flags: review?(philringnalda) → review+
fix checked in, thx, Phil.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact][has patch, needs review philor] → [no l10n impact]
You need to log in before you can comment on or make changes to this bug.