Closed Bug 20519 Opened 25 years ago Closed 25 years ago

[dogfood] key events getting through to editor key listener (symptom = control-v pastes text twice in HTML text controls)

Categories

(Core :: XUL, defect, P1)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: buster, Assigned: mjudge)

References

Details

(Whiteboard: [PDT+] [12/15])

html text fields now have a xul controller.
so commands issued by keybinding code now get to the editor via the controller
on the html text field content node.
the editor also has a key listener with some hard-coded bindings, to be used
when the editor is embedded in a XUL-less environment.
in the case of control-v (paste), the key binding code is issuing the command,
but not eating the key event.  so the editor key listener is getting the key
event, and 2 pastes happen.
hyatt says it should be a simple matter of the key binding code properly
consuming the key event if a command is dispatched.
this allows us to keep our hardcoded key binding in place as a default for
anyone to embed the editor without xul and still get basic operations.
Severity: normal → major
Priority: P3 → P2
Target Milestone: M12
I've set for M12 because I know this will come up as a usability issue.  let me
know if there's anything I can do to help.
Assignee: saari → hyatt
I'll take this.
Whiteboard: [PDT+]
Putting on PDT+ radar
*** Bug 20602 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Whiteboard: [PDT+] → [PDT+] 12/10 (fix will probably be in early next week)
Ok, I think I've got a way to get configurable key mappings without using
overlays... instead of using overlays, let's point to key binding files that
will get loaded and held by the XUL cache.

Then the key listener can look in those hardcoded files before it looks at the
key bindings in the main window.  This will give you configurable keys.
*** Bug 20744 has been marked as a duplicate of this bug. ***
hyatt:  I don't understand your comment on 12/2.  did you add that comment to
the right bug?
*** Bug 20621 has been marked as a duplicate of this bug. ***
*** Bug 20516 has been marked as a duplicate of this bug. ***
Assignee: hyatt → saari
Status: ASSIGNED → NEW
Assigning to saari, who can help out mjudge.  cc'ing waterson who can help.

The idea here.
(1) Add three XUL files to xpfe/global/resources/content
        textcontrolkeys.xul
        editorframekeys.xul
        browserframekeys.xul
These can be overlays (even though they won't really be being overlaid), that
contain the key binds.

(2) Modify the key listener to ask the command dispatcher (accessible from the
XUL doc) for the current focused object.  If it's an input field or textarea,
we'll look in textblah.xul.  If it's a DOM window, then depending on whether or
not it's in edit mode, we'll use one of the other two files.  mjudge can
probably tell how to query the shell to find out if it's in edit mode.

(3) After a profile is chosen, the three key bindings XUL files should be loaded
up (synchronously?) and put into the XUL cache.  waterson can help with
explaining how to do this.  They'd be in chrome://global/content/, but you want
to wait until a profile is determined to load them.

(4) The only tricky part.  The key bind code needs to execute and bubble up/be
capturably by the relevant immediate parent.  (e.g., the key event starts at the
input, textarea, or DOM window that's relevant).  Make sure that you don't let
the master key bind table get involved (we don't want it being handled twice).

(5) Once this is in place, Ender can remove all hardcoded key bindings.

(6) We make Gecko ship with XUL and RDF (yeah, baby, yeah, baby, yeah!).
Also, in case I wasn't clear, if the file doesn't contain a key binding for the
relevant key event, then it SHOULD go to the master key table (if there is one).
One other point: another thing waterson can help out with...
The three files should only make full-fledged XUL docs once from the
prototypes... they should then be kept around so that we don't end up making
them over and over again.
*** Bug 20781 has been marked as a duplicate of this bug. ***
Summary: [dogfood] key events getting through to editor key listener → [dogfood] key events getting through to editor key listener (symptom = control-v pastes text twice in HTML text controls)
added the symptom to the summary to make it obvious what problem this bug
addresses from the user's point of view.
*** Bug 20995 has been marked as a duplicate of this bug. ***
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Hyatt and myself landed all of this last night. Now, I don't know if editor has
removed their hard coded keybinds yet, but they can now. XPToolkit work is done
here.

Marking fixed because it makes me feel warm and fuzzy, but maybe this should go
back to editor guys
Status: RESOLVED → REOPENED
Assignee: saari → buster
Status: REOPENED → NEW
re-opening.  I agree that XPToolkit's work is done, but the editor team still
needs to remove the hardcoded key bindings. assigning to myself.
Status: NEW → ASSIGNED
Priority: P2 → P1
Resolution: FIXED → ---
Clearing FIXED resolution due to reopen.
Assignee: buster → saari
Severity: major → critical
Status: ASSIGNED → NEW
sending back to xptoolkit.
keystrokes simply aren't getting to text controls.  I comment out the hardcoded
keybindings in the editor, and I get no keybinding action in the browser URL
bar, content area text controls, address book chrome, etc.
I'm using code pulled late 12/9.
a bit more data...
it seems that I do get the key binding commands in the browser url bar *the
first time the url bar gets focus*  But if the url bar ever loses focus, it
never gets key events again.  html controls never get key binding commands, nor
did the chrome in address book.
Did you write the keybindings?  We didn't write them for you... you have to set
up your own keybindings in the URL bar and browser window....

I thought Mike Judge and Akkana were working on this...
Assignee: saari → mjudge
hyatt: that must be it.  I misunderstood, and thought that when saari checked
in, the editor keybindings were in as well.  Re-assigning to mjudge.
Mike, when you get the editor keybindings checked in, be sure to remove the
hardcoded keybindings in the editor event listener.  That part is trivial.  If
you already have a bug on the editor key bindings, you can dupe this one to
that.
Whiteboard: [PDT+] 12/10 (fix will probably be in early next week) → [PDT+] [12/15]
updating fix by datte
*** Bug 21492 has been marked as a duplicate of this bug. ***
Blocks: 21564
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
key binidngs are working now. ya!
Status: RESOLVED → VERIFIED
VERIFIED 1999121508 builds.
Note, xulkey-V in the urlbar still pastes twice -- see bug 21602.  I don't quite
understand what this bug was or why it wasn't a dup of that one, but the Summary
sounds a lot like 21602, that dom events aren't getting cancelled after XUL
keybindings act upon them and so are getting through to the editor event
listeners.
Blocks: 21602
It sounds like there's some confusion about editor event listeners vs. html text
control frame event listeners.
The editor has it's own event listener, totally independent of whether it's
wrappend in an html text control frame.  Any hardwired keybindings in this event
listener should be removed now that the keybinding stuff is in.
The html text control frame also has an event listener.  This includes no key
bindings.  It's used only to propogate events from the embedded webshell up to
any event listeners bound to the DOM content node <input type=text> or
<textarea>.  Without this, things like <textarea
onKeyDown=doSomethingInJavascript()> won't work, because the events don't bubble
up from the embedded webshell (which holds the editor).  In other words, events
don't bubble up past webshell boundaries without help.
Hope that helps.
Status: VERIFIED → REOPENED
So I made a mistake in verifying this. By my reading there's no way this could be fixed if
bug 21602 isn't. Based on buster's comments it would seem that's not the case however.
if someone could state a testcase that doesn't involve bug 21602 then I really appreciate
it and be glad to verify this bug properly. Reopening to clear the verified resolution.
Resolution: FIXED → ---
Clearing FIXED resolution due to reopen.
final m12 candidates are spinnning now. moving to m13.
if we fall off track and need to respin m12 for some
yet unknown reason we can consider this if you get
a fix in hand.
Blocks: 22176
Status: REOPENED → ASSIGNED
honestly this is in. if its not in m12 then it is definately in m13 saari fixed
this. I will wait to make sure
mike, I believe this was taken onto the M12 branch.  So you can mark it fixed,
and QA can verify against the branch and the tip if they like.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
yeah i will bite the bullet. this is fixed
Status: RESOLVED → VERIFIED
marking VERIFIED, mostly based on comments and bug 21602
No longer blocks: 21564
No longer blocks: 22176
You need to log in before you can comment on or make changes to this bug.