Shouldn't always unset shift when dispatching keypress event

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: Blake Ross, Assigned: Blake Ross)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
Rod, can you take a look at this patch?  Currently shift+space doesn't work
because of this; the shift gets ignored.  I think space is a special case
because shift+space doesn't produce a different character.  I couldn't think of
any similar cases to exclude.
(Assignee)

Comment 1

16 years ago
Created attachment 66613 [details] [diff] [review]
patch
(Assignee)

Updated

16 years ago
Blocks: 79047

Comment 2

16 years ago
What about function keys?

Comment 3

16 years ago
OK, I ran through a bunch of keys using my testcase in bug 50255.  Shift doesn't
get unset for function keys.  The only other keys I could finder that it did get
unset for were /*-+ on the keypad.  Is fixing this just a hack for the problems
in 50255?
(Assignee)

Comment 4

16 years ago
No, it gets unset for almost every key in the keypress event, which is what
we're dealing with here -- the reason being, I think, that shift+char wasn't
intended as some kind of accelerator, but just to uppercase (or otherwise shift)
char.  So it'd be weird to overload shift by both shifting the character and
passing shift as a modifier.

As you said, shift doesn't get unset for function keys.  That's because OnChar
is never even called for the function keys, and the system doesn't even seem to
post WM_CHAR or WM_SYSCHAR when you press a function key. So I'm not sure where
those get handled, but they do.

I don't really think this specific fix is much of a hack.  It makes sense not to
unset shift for space so we can handle shift+space in the app (which is what
this patch is for).  Now in the context of this function in general, from what I
read in 50255, this whole thing is a hack ;)
(Assignee)

Comment 5

16 years ago
Got r=ben and sr=hewitt on this and checked it in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

16 years ago
Hmm, I think I'm going to back this out.  With this fix, shift+space deosn't do
a space anymore sinec it's passed along as shift+space.  But without it, the app
can't handle shift+space. What is the answer here?

Comment 7

16 years ago
Did this get backed out?

Comment 8

16 years ago
If only event="keypress" keycode="VK_SPACE" worked... :-(

Comment 9

16 years ago
Forget that last comment, that conflicts with trapping space in checkboxes :-(

Hmmm... does that mean that even with a fix like this that checkboxes will have
to be fixed to work with the new shift+space as well as just space?

Updated

16 years ago
No longer blocks: 79047

Comment 10

16 years ago
*** Bug 104371 should have been marked as a duplicate of this bug ***
You need to log in before you can comment on or make changes to this bug.