122713 - QuickSearch: Single letter shortcuts are interfering with QuickSearch

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P1)

Description

[laurel]

Using jan30 commercial trunk build, Mac OS 10.1
Not seeing this with win98 or linux rh6.2

When typing text in the search text field which includes a "k", the Ignore
Thread command shortcut K appears to engage (I don't see that it is actually
ignoring threads). The search text is not echoed properly,it will be missing the
"k" and the search will not be executed as intended. 
 
Steps:
1.  Open a newsgroup.
2.  Select a message.
3.  (Search bar is shown) Type some text in the search field which includes a
"k".  Note the K is not echoed in the text field and the Message menu flashes.

Result:  Ignore Thread shortcut "K" is engaged when typing k in the search text
field (so far I don't see that the ignore really executes)

Comment 1

[laurel]

Same happens with Watch Thread shortcut "W".

Comment 2

[laurel]

I don't see this with Mac OS 9.2, just 10.1

Comment 3

[laurel]

Using today's commercial build on win98 I'm seeing this with W/Watch but not
K/Ignore. Strange.  So I guess we have a potential problem on different platforms.

Comment 4

[laurel]

Oh, and the win98 experience is actually marking the message Watched, where it
gets the icon and will display in a watched view.  Whereas Mac OS X would not
type the shortcut in the text field, the other platforms show the shortcut
letter typed in the text field AND then marks the message.  Since we had a bug
fixed since yesterday's build to show the icon immediately, it is now very
apparent whether the watch or kill command is enacted.

Comment 5

[scottputterman]

I'm seeing this on the 2/5 build on Win 2000.

Any key that is a shortcut is causing Quick Search to mess up:  't', 'n', 'f',
'a' all have this problem for me.

If I type one of these characters, a message gets selected, then Quick Search
occurs, then focus goes to the thread pane.  So, it's impossible to type a word
into the Quick Search bar that has one of these characters without having to put
focus back in Quick Search in order to complete the word.

Comment 6

[scottputterman]

'a' doesn't cause this. Thankfully we don't have that mark all messages any more.

Comment 7

[Navin Gupta]

This makes quicksearch completely useless. It doesn't seem to happen in old builds
I know for sure because i search for "naving" very often. I'll investigate. 

Comment 8

[Navin Gupta]

This is seen on 5th builds and not on 4th.

Comment 9

[Navin Gupta]

I backed out jst's checkins on 4th and 5th and that fixes the problem in my tree.
jst can you help us to narrow down this problem.

Comment 10

[Johnny Stenback (:jst)]

4th and 5th this month? This bug was reported before that? Or did you mean january?

Comment 11

[Johnny Stenback (:jst)]

Actually, I've been seeing this for a *long* time when signing in to IM in the
mailnews IM sidebar.

Comment 12

[laurel]

The bug here was originallly for K and W shortcuts.  The other single letter
shortcuts problem mentioned ("n" "t", etc.) wasn't a problem until the feb5th
build.  The IM sidebar issue was reported for label shortcuts awhile ago under
bug 120634.

Comment 13

[Navin Gupta]

4th and 5th this month (feb). This bug was reported on 30th Jan but 'n' key
interference w/ quick search is not seen after I locally backed your checkins. 

Comment 14

[Johnny Stenback (:jst)]

Exactly which checkins?

Comment 15

[Navin Gupta]

These are the checkins... you can look into bonsai for who -jst and time
between 2/4/2002 10:00 and 2/5/2002 12:00


# This page can be saved as a shell script and executed.  It should be
# run at the top of your CVS work area.  It will update your workarea to
# backout the changes selected by your query.

cvs update -j1.334 -j1.333 
mozilla/content/html/content/src/nsGenericHTMLElement.cpp
cvs update -j1.333 -j1.332 
mozilla/content/html/content/src/nsGenericHTMLElement.cpp
cvs update -j1.25 -j1.24 mozilla/dom/src/base/nsDOMClassInfo.h
cvs update -j1.91 -j1.90 mozilla/dom/src/base/nsDOMClassInfo.cpp
cvs update -j1.478 -j1.477 mozilla/dom/src/base/nsGlobalWindow.cpp
cvs update -j1.40 -j1.39 mozilla/js/jsd/jsd_xpc.cpp
cvs update -j1.301 -j1.300 mozilla/editor/composer/src/nsEditorShell.cpp
cvs update -j1.332 -j1.331 
mozilla/content/html/content/src/nsGenericHTMLElement.cpp
cvs update -j1.199 -j1.198 mozilla/content/base/src/nsDocumentViewer.cpp
cvs update -j1.55 -j1.54 mozilla/dom/src/base/nsJSEnvironment.h
cvs update -j1.173 -j1.172 mozilla/dom/src/base/nsJSEnvironment.cpp
cvs update -j3.47 -j3.46 mozilla/dom/public/nsIScriptContext.h
cvs update -j3.404 -j3.403 mozilla/content/html/document/src/nsHTMLDocument.cpp
cvs update -j3.352 -j3.351 mozilla/content/base/src/nsDocument.cpp
cvs update -j1.160 -j1.159 mozilla/dom/src/base/nsGlobalWindow.h
cvs update -j1.477 -j1.476 mozilla/dom/src/base/nsGlobalWindow.cpp
cvs update -j1.172 -j1.171 mozilla/dom/src/base/nsJSEnvironment.cpp
cvs update -j1.106 -j1.105 
mozilla/content/html/content/src/nsHTMLImageElement.cpp
cvs update -j1.118 -j1.117 
mozilla/content/html/content/src/nsGenericHTMLElement.h
cvs update -j1.39 -j1.38 mozilla/content/base/src/nsDocumentFragment.cpp
cvs update -j1.331 -j1.330 
mozilla/content/html/content/src/nsGenericHTMLElement.cpp

Comment 16

[laurel]

I also just checked a jan31 commercial win98 for sidebar interaction with the
"n" and "t" shortcuts -- no problem in that date's build, but there is in
feb06th. As mentioned, there's already a bug about label interference.  So
sidebar was affected recently for navigational shortcuts, will document in
separate bug just in case.

Comment 17

[laurel]

Follow-up for comment #16 -- new bug 123976 logged for sidebar case of these
shortcuts' interference.

Comment 18

[Johnny Stenback (:jst)]

Could you try backing out only version 1.331 of
mozilla/content/html/content/src/nsGenericHTMLElement.cpp, if that doesn't do
it, try backing out only the changes that went in on 02/04/2002 21:47. I don't
really see how any of those changes would cause anything this obscure, so we'll
need to narrow this down to the exact checkin and then start looking closer at
that...

Comment 19

[Navin Gupta]

jst, backing this out fixes the problem. 

cvs update -j1.331 -j1.330 
mozilla/content/html/content/src/nsGenericHTMLElement.cpp

Comment 20

[laurel]

*** Bug 124207 has been marked as a duplicate of this bug. ***

Comment 21

[Navin Gupta]

jst, how should we fix this ?

Comment 22

[Johnny Stenback (:jst)]

That fix is a valid and good fix. What it does is to change element.localName to
be lowercase on XHTML elements in stead of being upper case. If you search for
".localName" in the code that deals with the shortcuts n' all that I bet you'll
find code that does things like:

  if (element.localName == "A") ...

Change that to lowercase the string before comparing and the problems that were
caused by the above check will go away.

Comment 23

[Navin Gupta]

*** Bug 123976 has been marked as a duplicate of this bug. ***

Comment 24

[Navin Gupta]

jst, thanks for the info. fix upcoming

Comment 25

[Navin Gupta]

Attachment: proposed fix

The fix is to change "input" and "textarea" to lower case as jst points out. 

sean, can I get r=

Comment 26

[Sean Su]

Comment on attachment 68607 [details] [diff] [review]
proposed fix

r=ssu

Comment 27

[Scott MacGregor]

Comment on attachment 68607 [details] [diff] [review]
proposed fix

sr=mscott

Comment 28

[Navin Gupta]

fixed

Comment 29

[Johnny Stenback (:jst)]

Um, I don't know exactly where this code is and what it affects, if we know that
the element in question (focusedElement) is always a XHTML element, then this is
fine, but if it could just as well be a HTML element then you'd need to worry
about localName being "input" in some cases and "INPUT" in other cases.

If we know we're dealing with XHTML all the time here, then this is a fine fix,
if not we should make |name| be |focusedElement.localName.toLowerCase()|.

Comment 30

[Navin Gupta]

Attachment: converting to lower case

better to be safe. jst, can you sr.

Comment 31

[Johnny Stenback (:jst)]

Comment on attachment 68639 [details] [diff] [review]
converting to lower case

sr=jst

Comment 32

[Navin Gupta]

thanks, fix checked in. 

Comment 33

[laurel]

Using feb11 commercial trunk: win98, linux rh6.2 (assume mac, too...haven't
gotten that far) both the sidebar and search bar still have a problem with "w"
for watch thread.

Comment 34

[Navin Gupta]

Attachment: proposed fix

fix for 'w' - watch thread.

Comment 35

[Sean Su]

Comment on attachment 68942 [details] [diff] [review]
proposed fix

we should not place the check in supportsCommand function, but rather in the
isCommandEnabled function like all the other ones.

Comment 36

[Navin Gupta]

Attachment: fix

right, I intended it to be that.

Comment 37

[Sean Su]

Comment on attachment 68945 [details] [diff] [review]
fix

r=ssu

Comment 38

[Scott MacGregor]

Comment on attachment 68945 [details] [diff] [review]
fix

sr=mscott

Comment 39

[Navin Gupta]

fixed

Comment 40

[laurel]

OK using feb18 commercial trunk build: win98, mac OS 10.1, linux rh6.2

Comment 41

[Pekka Nikander]

I saw a bug very similar to this one today on 2002081011 Moz 1.1 FC build,
under MacOS X 10.1.5.  Basically, the single letter shortcuts M and R
would not work.  They would not get entered to the quicksearch box, and
the Message menu was briefly highlighted.  With some tinkering it appeared
that it wasn't this bug after all, but Mozilla was somehow confused about
focus.  That is, even though keyboard focus was clearly on the QuickSearch
box (since all the other letters were included there), it was still somehow
thinking that focus was on some message so that M and R would have been
legal commands...

I wasn't able to reproduce, and since there are so many of the focus issues
anyway, I don't think this is worth opening a new bug...