129721 - Filter UI: Enter while focus is on New/Edit/Del/Move button will close dialog.

Status: RESOLVED WORKSFORME

(MailNews Core :: Filters, defect)

Description

[laurel]

Using mar08 commercial trunk, win98 & linux rh6.2

I thought maybe this was logged a long time ago, but didn't find it. Could be a
mention in some older cluttered bug.

Now that we're implementing more accessibility and just recently in filters (bug
40761) this may be a bit more visible.

When tabbing or otherwise focus placed on any of the action buttons (New, Edit,
Delete, Move Up, Move Down) in the main filter list dialog, if you hit Enter the
dialog is dismissed instead of opening the filter rules (for New or Edit) or
doing appropriate action (for Delete,Move Up/Down).

Steps:
1.  Edit|Message Filters.
2.  Tab to focus on the New button.  Press Enter.

Result:  filter ui dismissed.

Expected:  filter rules dialog should open.

Comment 1

[Håkan Waara]

I'm looking into this... this is due to the OK button on the window taking control.

Comment 2

[Håkan Waara]

Attachment: Patch

dialogOverlay.xul is deprecated, and all new windows/dialogs should use
<dialog/>. This focus bug is fixed in <dialog/>, so the fix is to convert
FilterListDialog (FilterEditor is already converted).

Comment 3

[(not reading, please use [e-mail] instead)]

your changes make me worry that you broke some of the pre-fill-filter feature.

cc varada, who would better than me.

Comment 4

[Håkan Waara]

I tried "Create Filter", and it worked.  Can you give me a testcase that you
think I broke?

Comment 5

[Håkan Waara]

Worked from the context-menu on message headers as well.

Comment 6

[Håkan Waara]

I spoke to varada over AIM, and asked what tests I should perform to ensure I
didn't break anything.  Result: everything works fine, no regressions found.

I'll continue searching for a reviewer.

Comment 7

[Joe Hewitt (gone)]

Comment on attachment 73260 [details] [diff] [review]
Patch

You don't need to call window.close anywhere, the dialog handles that
automatically.	So, you don't really need the ondialogcancel handler at all. 
Remove that por favor, and then count the sr=hewitt

Comment 8

[Håkan Waara]

Attachment: Updated patch

Patch with hewitt's comment adressed.

Per request, here are the list of things to test varada gave me over AIM:

1. Right-click on a mail-address and Create Filter.
2. Choose "Create Filter" from the Message menu on a selected message.
3. Make sure the "OK" and "Cancel" buttons work alright in the Filter Editor
after "Create Filter".

Comment 9

[Håkan Waara]

Comment on attachment 74317 [details] [diff] [review]
Updated patch

Carry sr=.

Comment 10

[(not reading, please use [e-mail] instead)]

I tried this patch, and it looks like it breaks pre-fill filter.

right click on a email addresses in the header area of the message pane, do
"Create Filter"

taking from naving, since he's on vacation.

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 74317 [details] [diff] [review]
Updated patch

this breaks a the pre-fill filter feature.

Comment 12

[Håkan Waara]

I swear that this worked when I tried it... :-(

Comment 13

[Mike Cowperthwaite]

This problem appears to have been corrected, 1.5-Final, Win2K.

Comment 14

[Mike Cowperthwaite]

No response... => WFM