27908 - undo undoes twice; paste pastes twice (keybinding)

Status: VERIFIED FIXED

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

Description

[sujay]

using 2/15 build of mozilla

1) launch mozilla
2) launch editor
3)enter some text
4) make it bold
5) Ctrl-Z

the entire text disappears..I was expecting it to revert to normal text, not
erase everything..

all platforms.

Comment 1

[Kathleen :Brade]

keybindings are being triggered twice (two undos are happening).  This seems like 
a duplicate of another bug (possibly one that needs to be reopened).

The menu "undo" doesn't work at all; I assume that is new and unrelated to this 
problem.

Comment 2

[Kathleen :Brade]

I think this is a conflict with the editor event listeners which have hard coded 
keybindings; reassign to myself; cc akkana

Comment 3

[Akkana Peck]

Possibly, but pasting with the mouse button also pastes twice (bug 23336) and
that's ONLY handled by the editor event listener, as far as I know.

Comment 4

[Akkana Peck]

There was already a bug on this, bug 26381, that hyatt marked fixed, though the
comment says something like it will be fixed after joki changes something in the
dom event model.  Perhaps it should be reopened?  Adding hyatt (who owned that
bug) to the cc list.

Comment 5

[David Hyatt]

joki added this, and in theory I fixed this.

Comment 6

[Kathleen :Brade]

this is broken again in my build from today (Mac).  Akkana can't reproduce on 
Linux (paste) except for mouse events.

Comment 7

[Kathleen :Brade]

pass to hyatt to track down why editor event listeners are getting called before 
xul keybindings

Comment 8

[Simon Fraser [no longer active]]

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

Comment 9

[Ninoschka Baca]

With the 2/16/2000 builds while testing Composer/Mail and then trying the same 
scenarios in Editor I found:

- After a Cut or Copy, it appears that Paste using the menu item is working. The 
problem is with the shortcut, Ctrl+V, which is duplicating text.
- For Undo, it appears the menu item is also working but the problem with undo 
twice occurs when using the shortcut, Ctrl+Z (Win and Mac). On Linux Alt+Z does 
nothing but Ctrl+Z works.

Comment 10

[Kathleen :Brade]

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

Comment 11

[Kathleen :Brade]

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

Comment 12

[Akkana Peck]

Kathy and Hyatt: in looking for the workaround to bug 23336, I found that the
ender event listener (in nsGfxTextControlFrame.cpp) seems superfluous on mouse
click events; if I return NS_OK from nsEnderEventListener::MouseDown rather than
handling it there, the event passes through to the editor event listener and
everything works right, and we didn't need the ender event listener stage at all.

You might want to try the same thing for key events in the ender event listener.
 The more I look at it, the more I wonder why the ender event listener is there
at all.

Cc'ing buster in case he can provide some background on why the listener was put
there in the first place rather than just letting events be handled by the
regular editor event listener.

Comment 13

[buster]

some quick background:
when ender was installed as the text control, there was no automatic propogation 
of events from a document to an embedded webshell.  thus, the ender event 
listener was created to take an event targeted at the text control frame and 
retarget it down to the webshell contained by the text control frame.  thus the 
editor would get the event, where without this step it would not.
originally, this was just done for keyboard events.
as the implementation of focus mutated, at times it was required to propogate 
forcus events as well.
to get selection-on-first-click behavior for text controls that are lazily 
instantiated, the mouse event code was added.

I added this same comment to 23336.

Comment 14

[Kathleen :Brade]

add some keywords, cc bijals

Comment 15

[Phil Peterson]

PDT+

Comment 16

[Akkana Peck]

I just tried this out on Mac, and I saw it in the editor window, but nowhere
else.

I assume this is coming from our hardcoded keys in ProcessShortCutKeys().  But
we don't need those keys any more now that text fields and text areas can usguts
of ProcessShortCutKeys?

I tried this on Mac: I inserted "return NS_OK" after line 230 of
nsEditorEventListener::ProcessShortCutKeys (i.e. just after it sets
aProcessed=PR_FALSE), and now I got single pastes everywhere -- in the editor
window, in the urlbar, and in text fields and text areas inside a bugzilla page.

I recommend that we nuke ProcessShortCutKeys.  It has outlived its usefulness. 
I'll volunteer to check this in tomorrow, if someone else wants to review it. 
Assign the bug back to me if you want to take this route.

Comment 17

[David Hyatt]

agreed.

Comment 18

[Kathleen :Brade]

M14
Akkana--we should just remove that method altogether.  I tested this on Mac also 
and I agree that it will work.

HOWEVER, I am still concerned that the flow of events has changed.  We may see 
other problems caused by this change in the flow of events.  It seems to me that 
the event should've been "handled" by a keybinding and the editor shouldn't be 
getting the event.  I'd really like to see that get fixed before we remove this 
method.

Comment 19

[Akkana Peck]

Accepting.  Kathy, have you looked at these changes yet?  Do you agree that it's
the right thing?  I'd love to get heavier testing on Mac and Windows (I'll run
with it on Linux today, and will test it on Windows as well) and a code review.

Comment 20

[Akkana Peck]

(oops, I missed Kathy's comment when I made mine.)
Awaiting PDT approval to check in the fix.

Comment 21

[Akkana Peck]

Checked in.

Comment 22

[laurel]

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

Comment 23

[sujay]

verified in 2/22 build.