128647 - [RFE] Handler for WM_COPY/WM_CUT/WM_PASTE/WM_CLEAR

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P4)

Description

[Koike Kazuhiko]

Mozilla doesn't have handler function for WM_COPY/WM_CUT
/WM_PASTE/WM_CLEAR. But standard windows application should
have them.

Comment 1

[Mike Kaply [:mkaply] (Out until 9/29)]

We can do this real easy. Just handle the right messages and send the
appropriate NS_KEYPRESS. I got this working on OS/2.

Comment 2

[Pham]

.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

ATOK(Japanese IME) users want to fix this bug.

Because WM_COPY is used for adding new word to dictionary.
This bug is inconvenience for Japanese ATOK users.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

VJE-Delta ver 4.0 that is IME for JP. It is using WM_COPY for its reconverting.
So, we need to implement this. Therefore I change Severity "enhancement" to
"minor". 
# The reason of "minor" is that VJE-Delta is minor IME in Japan.

Comment 5

[Wayne Mery (:wsmwk)]

bumping to major to match bug 301205

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

this patch works fine for me, but needs more some works.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.0

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.0

This adds to support the clipboard/editing related messages such as WM_CUT, WM_COPY, WM_PASTE, WM_CLEAR, EM_UNDO, EM_REDO, EM_CANPASTE, EM_CANUNDO and EM_CANREDO.

I added nsContentCommandEvent for them and we will be able to use it in other bugs (I need to request the cursor move without key events in other bugs). So, I think this name is good for the previous name (nsEditEvent).

The events are handled in nsEventStateManager. The ESM gets the command controller for the requested action and checks whether the command is enabled or not. And also it executes the command if it's requested.

This patch also moves the event type checking in nsViewManager and nsPresShell to the inline functions in the nsGUIEvent.h. This helps to add some new events. And this guarantees the consistence of the conditions between them. Especially, they are using different logic on current trunk for checking whether the event is dispatched at the coordinates.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 391535 [details] [diff] [review]
Patch v3.0

oops, I didn't check the automated tests on tryserver. Pending the sr request but the Win32 part can be reviewed.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #8)
> I added nsContentCommandEvent for them and we will be able to use it in other
> bugs (I need to request the cursor move without key events in other bugs). So,
> I think this name is good for the previous name (nsEditEvent).

I meant that the name is better than the previous name (nsEditEvent).

Comment 11

[Ere Maijala (slow)]

Comment on attachment 391535 [details] [diff] [review]
Patch v3.0

r+ for the Windows widget part.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.0.1

Thank you, Ere.

Comment 13

[Peter Weilbacher]

Is there any case outside IMEs where this is useful? I mean, should we implement this for OS/2, too?

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #13)
> Is there any case outside IMEs where this is useful? I mean, should we
> implement this for OS/2, too?

I'm not sure. But there are some possibilities: e.g., some utility software may access to the content of the focused application by these messages. So, this messages can improve the accessibility from other applications. The risk and the cost by this implementation are small, therefore, if you can implement this easily, I don't have any objections. But it shouldn't be major issue.

Comment 15

[Peter Weilbacher]

OK, I discovered that the relevant WM_ messages are only available in the Win32 compatibility layer of OS/2. I very much doubt that any applications exist (which are still useful today) that make use of them. So I won't do an OS/2 implementation after all.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    aEvent->mIsEnabled = canDoIt ? 1 : PR_FALSE;

Why not just aEvent->mIsEnabled = canDoIt?

+  aEvent->mSucceeded = 1;

PR_TRUE?

+inline PRBool NS_IsUsingCoordinatesEvent(nsEvent* aEvent)

NS_IsEventUsingCoordinates would probably be better

+inline PRBool NS_IsFocusedShellTargettedEvent(nsEvent* aEvent)

NS_IsEventTargetedAtFocusedWindow would probably be better

I wonder if we should just use nsCommandEvent --- and extend it with the "enabled" flags --- instead of creating a new event type?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #16)
> +    aEvent->mIsEnabled = canDoIt ? 1 : PR_FALSE;
> 
> Why not just aEvent->mIsEnabled = canDoIt?

Can I use PRBool value to PRPacked variable? Yes, I know we can use it actually. But I'm not sure whether we should do it or not so.

> I wonder if we should just use nsCommandEvent --- and extend it with the
> "enabled" flags --- instead of creating a new event type?

nsCommandEvent is handled in browser.js, but the new events cannot be handled by it. They should be handled by ESM or something. And they don't need to fire their DOM events. So, it seems that nsCommandEvent using creates a new different handling path of it. Don't other developers confuse by it?

I think the name of nsCommandEvent is wrong for its current meaning. It should be nsNavigationCommandEvent or something. And the new event should be nsCommandEvent. But we should not rename them....

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #17)
> (In reply to comment #16)
> > +    aEvent->mIsEnabled = canDoIt ? 1 : PR_FALSE;
> > 
> > Why not just aEvent->mIsEnabled = canDoIt?
> 
> Can I use PRBool value to PRPacked variable? Yes, I know we can use it
> actually. But I'm not sure whether we should do it or not so.

You should. The only legal values in PRBool are 1 or 0. So your code here doesn't make any difference.

> I think the name of nsCommandEvent is wrong for its current meaning. It should
> be nsNavigationCommandEvent or something. And the new event should be
> nsCommandEvent. But we should not rename them....

OK, I'm convinced :-)

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.1

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 393130 [details] [diff] [review]
Patch v3.1

+    aEvent->mIsEnabled = PRPackedBool(canDoIt);

Don't need the cast.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.2

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://hg.mozilla.org/mozilla-central/rev/9c37b8a909a2

Comment 23

[Jacek Caban]

Attachment: Use lower case in includes to fix compilation on mingw.

This patch breaks cross compilation on Linux mingw by using mixed case in includes. The attached patch fixes it (it also fixes oleidl.h includes that I didn't notice earlier because it's ACCESSIBILITY ifdefed).

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 393358 [details] [diff] [review]
Use lower case in includes to fix compilation on mingw.

looks ok, but you need to change widget/tests/TestWinTSF.cpp too.

Comment 25

[Jacek Caban]

Attachment: Use lower case in includes to fix compilation on mingw.

You're right, thanks. Attached patch includes test fix.

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 393362 [details] [diff] [review]
 Use lower case in includes to fix compilation on mingw. 

Thank you for your work!

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://www.mozilla.org/hacking/reviewers.html

Oh, we don't need sr+ for such simple fix now?

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 393362 [details] [diff] [review]
 Use lower case in includes to fix compilation on mingw. 

no, this doesn't need sr

Comment 29

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/bc6ffc794fc1