Closed Bug 6896 Opened 25 years ago Closed

[BLOCKER] Windows editor interpres the '.' key as a forward delete

Categories

(Core :: DOM: Editor, defect, P2)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: tague, Assigned: tague)

References

Details

(Whiteboard: patch checked-in [conditionallized] waiting to be turned on)

Attachments

(1 file)

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;
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.
Attached patch patch (deleted) — Splinter Review
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.
*** Bug 6907 has been marked as a duplicate of this bug. ***
Assignee: kostello → joki
Rod's on vacation for a few weeks, so assigning to Pixly to see if he'll bite. Tom?
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.
*** Bug 6442 has been marked as a duplicate of this bug. ***
Note that bug 6442 contains a list of characters that don't work in the editor on Windows.
patch is related to #4816
Blocks: 4816
Blocks: 6053
Blocks: 7470
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.
Blocks: 7445
Blocks: 7577
Blocks: 7228
Assignee: joki → tague
Since tague seems to be working on a fix for this and I'm not, reassigning.
Depends on: 7692
Depends on: 7629
No longer depends on: 7692
Status: NEW → ASSIGNED
Target Milestone: M7 → M8
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.
nope
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
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.
<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.>
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.
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.
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
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.
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.
Target Milestone: M8 → M7
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.
Blocks: 6262
Whiteboard: patch checked-in [conditionallized] waiting to be turned on
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.
Turned on fix today (6/13/99; 2:30pm). Should be in Monday's verification build.
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.
Status: RESOLVED → VERIFIED
verified in 6/16 build.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: