Closed
Bug 6896
Opened 26 years ago
Closed
[BLOCKER] Windows editor interpres the '.' key as a forward delete
Categories
(Core :: DOM: Editor, defect, P2)
Tracking
()
VERIFIED
FIXED
M7
People
(Reporter: tague, Assigned: tague)
References
Details
(Whiteboard: patch checked-in [conditionallized] waiting to be turned on)
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Steps to reproduce :
(1) launch build
(2) open editor
(3) click in window and start typing periods. it will act as a forward delete
What the problem is :-
nsEventListener.cpp is processing key downs incorrectly. The problem is that,
nsKeyDown is handling both virtual key and character events on the same handler.
It is comparring the virtual keycode for VK_DELETE with the character code for
'.', which happen to be the same.
if (PR_FALSE==keyProcessed)
{
switch(keyCode) {
case nsIDOMUIEvent::VK_BACK:
mEditor->DeleteSelection(nsIEditor::eDeleteLeft);
break;
case nsIDOMUIEvent::VK_DELETE:
mEditor->DeleteSelection(nsIEditor::eDeleteRight);
break;
Comment 1•26 years ago
|
||
The problem seems to be that char codes are being put in the virt-key code
slot. I believe that would make this a widget bug.
The problem is that people are mixing virtual key events and character events.
The attached patch fixes this problems, plus a few others with non-Unicode input
on Windows. Its been lightly tested on Windows, and these changes need to be
synched with both Mac and Unix.
The patch affects both Ender (nsEventLister.cpp) and Widget (nsWindow.cpp).
Priority: P3 → P2
Summary: Windows editor interpres the '.' key as a forward delete → [BLOCKER] Windows editor interpres the '.' key as a forward delete
Target Milestone: M7
I've marked 6907 a dup of this, and made this one a [BLOCKER]. Ender cannot act
as the text control until this is solved, because no one will be able to type a
URL into the URL bar without a '.' I'm sure there are other keys that are
behaving badly due to this as well.
Marked P2, M7. Assigned to Rod, since no one else seems to want it. Rod, can
you either verify Tague's changes and check them in, or get him to check them
in, or find the right person to do it? I'll happily do the editor part of it
once basic event creation is fixed.
Bug 6907 contains some additional detail.
Rod's on vacation for a few weeks, so assigning to Pixly to see if he'll bite.
Tom?
Comment 7•26 years ago
|
||
I thought tague checked in a fix for this. Tague..?
simon :- no i didn't check in the change. it's a rather big change that needs
to be synched up with both Mac and Linux as well. i also wanted more people to
look at it first, because I think the selection mechanism may be incorrectly
looking at virtual keys instead of character events.
this patch needs a bit more QA and a carpool before it's ready to check in.
Comment 10•26 years ago
|
||
Note that bug 6442 contains a list of characters that don't work in the editor
on Windows.
Assignee | ||
Comment 11•26 years ago
|
||
patch is related to #4816
Comment 12•26 years ago
|
||
We will clear out a lot of bugs when the fix for this is checked in. Tague,
what is your plan for getting this in?
Added 7470 to the dependancy list.
Updated•25 years ago
|
Assignee: joki → tague
Comment 13•25 years ago
|
||
Since tague seems to be working on a fix for this and I'm not, reassigning.
Comment 14•25 years ago
|
||
Is there any way to get this fixed for M7? Editor (esp mail compose) is not
really useful if we can't type punctuation marks, etc, in it. Look at all the
bugs depending on this bug. Thanks.
Assignee | ||
Comment 15•25 years ago
|
||
nope
Comment 16•25 years ago
|
||
M7 got another week. Why isn't that enough to get this fixed? I'm curious, not
**** off (well, that "nope" bugged me a little). Would it help if more people
applied the patch? Do you need help on other platforms? Are there owners back
from vacation now?
/be
Comment 17•25 years ago
|
||
I want to be sure we understand just how big a blocker this bug really is. This
bug seriously impacts several areas:
1. We can't turn on GFX-rendered widgets until this bug is fixed. That means
everyone has to suffer with the native widget bugs for another release.
Engineers (mostly) are not fixing native widget bugs in anticipation of
GFX-rendered widgets.
2. The editor is crippled.
3. Mail compose is crippled.
The editor team has offered to supply a resource here to help, haven't gotten
any feedback about that offer.
Comment 18•25 years ago
|
||
<And mail QA has offered to test special builds, if needed, with the fix in it
and verify all the mail dependent bugs to see any major regressions show up.>
Assignee | ||
Comment 19•25 years ago
|
||
the windows code is already checked into the tree and can be tested.
to test it, you need to do a build with tague_keyboard_patch defined in your
environment.
You can set this variable using "set CL=/Dtague_keyboard_patch=1"
VC++ looks at the CL variable for additional command line options.
The changes are localized to the widget, layout, and editor directories. You
can just do a clobber build in these three directories and it should work.
Comment 20•25 years ago
|
||
Tague - do you have an optimized build that you've already done that I can have?
I don't have build or debug environment that I can use.
Assignee | ||
Comment 21•25 years ago
|
||
linux is checked in and ready. linux right now will only support roman/latin-1
input, unlike windows. i don't have the support infrastructure yet to do real
unicode translation in the keyhandler. the unicode stuff can be added later.
to build add "
OS_CFLAGS += -Dtague_keyboard_patch=1
OS_CXXFLAGS += -Dtague_keyboard_patch=1
"
to mozilla/config/myconfig.mk
Assignee | ||
Comment 22•25 years ago
|
||
prebuilt linux and win32 binaries are availible behind the firewall in
/u/tague/pub/seamonkey. i have no objection to them being pushed outside the
firewall if QA wants to make the appropriate arrangements.
Comment 23•25 years ago
|
||
Starting tomorrow, I'll have a few people use the Linux and Win32 builds you've
made (thanks for doing so) to check out the dependent bug reports. We'll also
go through our basic mail test to see if any major regressions have occurred.
Will post our results in the early afternoon tomorrow in this bug report.
International QA may want to do some testing as well. I don't know all the
implications of this bug fix besides from the mail point of view. If all is
ok, then perhaps we can get this fix turned on for M7 and get a few days of
wider usage afterwards. Thanks.
Comment 24•25 years ago
|
||
Tested this scenario with experimental build on En Win NT4.0 system in IQA lab.
The scenario mentioned in this bug is fixed. The Basic Smoke test for mailnews
passed too. FYI...backspace now does not work in the Addressing fields with this
special build, have not noticed that in QA released builds so I will wait to log
a bug for that.
Whiteboard: patch checked-in [conditionallized] waiting to be turned on
Assignee | ||
Comment 25•25 years ago
|
||
update status. patch in on all 3 platforms; only know problem is that the
down-arrow won't work. joki can help us clean that up on Monday.
my plan is to turn this on before Monday's verification build unless anyone has
objections.
Assignee | ||
Comment 26•25 years ago
|
||
Turned on fix today (6/13/99; 2:30pm). Should be in Monday's verification
build.
Comment 27•25 years ago
|
||
Looks good in the html compose window on Win32 6/14 release builds. I've been
using the various punctuation keys in the mail compose window and trying to use
mail 5.0 for "real" work.
Comment 28•25 years ago
|
||
verified in 6/16 build.
You need to log in
before you can comment on or make changes to this bug.
Description
•