selection is moved when using a keyboard shortcut with focus on left pane

VERIFIED FIXED

Status

()

Core
XUL
VERIFIED FIXED
16 years ago
5 hours ago

People

(Reporter: Nahor, Assigned: Kyle Yuan)

Tracking

({regression})

Trunk
x86
All
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
With build 2002051808 from trunk, when using a shortcut and when the focus is on
the left pane (list of mail folder/newgroups), the selection changes to the next
folder/group starting with the letter of the shortcut.

Steps:
  1. have a news group starting with f (e, v, g, m, t, w and h work too).
  2. Have this group/folder visible (i.e branch expanded)
  3. Select any folder/groups other than the one from step 1.
  4. Press "alt+f" to open the file menu (or one of the other menu shortcut
depending of your choice at step 1.)

result:
  The menu is open and the selected folder/group changes to a folder/group
starting with letter f (e,v ... respectively).

expected result:
  Menu opens but selection is *not* moved.

Alternatives:
  - Try to do a search for a particular mail/news (ctrl+shift+f) and the
selection moves to the folder/groups whose first letter is a f.
  - "Mark all read" (ctrl+alt+c) has the same issue.

Remarks:
  Moving the selection when a letter is pressed is a nice feature but the
selection should not moved if a modifier (alt or ctrl) is pressed (I think shift
would be ok).

Comment 1

16 years ago
confirmed with Linux build 2002051808 (trunk).

regression between 2002051507 (trunk) and 2002051621 (trunk)

probably bug 133366

cc'ing Kyle Yuan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Windows 2000 → All

Comment 2

16 years ago
*** Bug 145575 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Summary: selection is moved when using a shortcut with focus on left pane → selection is moved when using a keyboard shortcut with focus on left pane

Comment 3

16 years ago
this also occurs in Javascript Debugger and DOM Inspector

==> Trees (Menus?)
Assignee: sspitzer → hewitt
Component: Mail Window Front End → XP Toolkit/Widgets: Trees
Product: MailNews → Browser
QA Contact: olgam → shrir
(Reporter)

Comment 4

16 years ago
For notice, there is a similar problem (bug 145403) in MailNews with shortcuts
that don't have a modifier (shortcuts in go menu: f, n, t, b and p)
(Assignee)

Comment 5

16 years ago
this regression is caused by bug 133366. taking...
Assignee: hewitt → kyle.yuan
(Assignee)

Comment 6

16 years ago
Created attachment 84213 [details] [diff] [review]
patch


1. Disable the keynavigation feature in mailnews' folderpane, because there are
some single letter shortcut key in mailnew module (B N T F ...)
2. Filter out the keypress that combined with alt, ctrl, shift, meta
(Assignee)

Comment 7

16 years ago
asking Jan for r= ...

Jan, you are right. We still need to disable this feature in folderPane.xul.
Status: NEW → ASSIGNED
(Reporter)

Comment 8

16 years ago
-         else if (!this.disableKeyNavigation && event.charCode > 0) {
+         else if (!this.disableKeyNavigation && event.charCode > 0 &&
+                  !event.altKey && !event.ctrlKey && !event.shiftKey &&
!event.metaKey) {

Do we really want shift to disable navigation?
Because to me, like <letter> shortcuts, <shift>+<letter> shortcuts should not be
used. Too likely to be a problem if a textfield appears in the window that
didn't use to have one.
And on case-sensitive systems, people maybe enclined to use shift if an item of
the tree starts with a capital.
(Assignee)

Comment 9

16 years ago
Nabor: 
1. I think <shift>+<letter> is supposed to be a combine shortcut, even we 
haven't used them yet.
2. In key-navigation code, I use *case-insensitive* string compare.

Comment 10

16 years ago
Comment on attachment 84213 [details] [diff] [review]
patch

r=varga
Attachment #84213 - Flags: review+
(Reporter)

Comment 11

16 years ago
Kyle:

1. I guess. If <letter> can be used as a shortcut, there is no reason that
<shiht>+<letter> would not be. But to me, that's seems to be a bad design that
can cause problem later. <ctrl> and <alt> are good for shortcuts because they
are not used to type normal text, but <shift> is.
2. I know, I wasn't talking about changing the search into a case-sensitive case
but to allow people to type a name as it is written on screen. It's not really
an important issue, it's just one of those little details that may make life
easier. :)
Comment on attachment 84213 [details] [diff] [review]
patch

sr=jst
Attachment #84213 - Flags: superreview+
here's what bienvenu reported:

when the folder pane has focus, if I hit "n" (for next unread folder), it loads 
a folder that starts with n.

this was caused by the checkin for #145532.

the mailnews part of the attached patch fixes this problem.

I've landed the mailnews part, now to land the xpfe part since it has r/sr.
*** Bug 145403 has been marked as a duplicate of this bug. ***

Comment 15

16 years ago
*** Bug 145738 has been marked as a duplicate of this bug. ***
sr=sspitzer on the mailnews part.

both mailnews and xpfe fix landed on the trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 17

16 years ago
*** Bug 145847 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 18

16 years ago
Build 2002052104

- The Mail/News is fixed (no more navigation)
- I tried navigation on the bookmark window and the preference dialog. It works
as I would like it to work but not as the patch intended it. <ctrl> and <alt>
disable the navigation but I can still navigate using <shift>+<letter>.

So, again, I'm not complaining about the behavior, I like it that way, but if
the patch above has been commited "as is", either "event.shift" is
never/incorrectly set or something shunt the test somehow.
(Assignee)

Comment 19

16 years ago
Nahor, thanks for your careful testing. 
I found that event.shiftKey is true only when you pressed <ctrl>+<shift> on the 
same time. It's right your desire :)
(Reporter)

Comment 20

16 years ago
Does it need to be filed as a new bug or is it bug 104371 (or maybe bug 126189)?
(Assignee)

Comment 21

16 years ago
yes, those two bugs are aimed at this problem, let me checking...
(Assignee)

Comment 22

16 years ago
See:
GTK:
http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsGtkEventHandler.cpp#400
Windows:
http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#2832

Seems the designer want to turn shift off when you pressed <shift>+<ascii key>. 
I don't think this is a bug.

(Reporter)

Comment 23

16 years ago
In Mail/News, shortcut <n> (select next unread message) is different from
<shift>+<n> (do nothing) but with your patch the two are the same? Go figure...
But well, I'm just a free loader of Mozilla :p... In all case, the bug fix is
closed and verifed. This argumentation should go into another bug or one of the
above.
Thanks Kyle. :)
Status: RESOLVED → VERIFIED
(Assignee)

Comment 24

16 years ago
I don't think we are arguing, this is a helpful discussion :)

The fact in mailnews is:
If you turned off CapsLock, <n> works, but <shift>+<n> doesn't;
If you turned on CapsLock, <shift>+<n> works, but <n> doesn't;
because <n> is the shortcut key, but <N> isn't.


(Reporter)

Comment 25

16 years ago
Ok then, lets go on *grin*:

But whatever the status of capslock, if you want to mark all mail as read, you
have to press <ctrl>+<shift>+<c>. And from the gtk link you gave, it says "make
Ctrl+uppercase functional as same as Ctrl+lowercase".
So <n> and <N>(capslock) should behave the same way and <shift>+<n> and
<shift>+<N> too.
In other word, shortcuts should be based on keycodes and not on ascii codes.

Updated

10 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets

Updated

5 hours ago
Blocks: 1455391

Updated

5 hours ago
No longer blocks: 1455391
You need to log in before you can comment on or make changes to this bug.