Mouse clicks in inline input areas are not forwarded to input methods.
Steps to reproduce:
1. Open WebCore/layout-tests/editing/inserting/typing-001.html
2. With any input method that uses inline input (such as Kotoeri Hiragana), type something
3. Click in the middle of the inline input area
The insertion point is correctly positioned where you have click; the input area doesn't close
4. Continue typing
Results: text is appended to the end of the inline input area
Discussion: I believe this happens because mouse events are not being correctly sent to TSM.
(In reply to comment #0)
> Mouse clicks in inline input areas are not forwarded to input methods.
> Discussion: I believe this happens because mouse events are not being correctly sent to TSM.
Yep, this is definitely a problem. There is a comment in -mouseDown:
// TEXTINPUT: if there is marked text and the current input
// manager wants to handle mouse events, we need to make sure to
// pass it to them. If not, then we need to notify the input
// manager when the marked text is abandoned (user clicks outside
// the marked area)
But I can't see where the above is actually being done. Some portion of the above functionality IS
provided when _selectionChanged is called. However, what happens in _selectionChanged is not totally
correct - the actual mouse down event does not actually get passed through to TSM (and therefore to
the input method itself).
The impression is given that the line of code that follows the above comment does this:
[_private->compController endRevertingChange:NO moveLeft:NO];
But it's solely for dealing with completion, and has nothing to do with the above comment.
So it looks like there is some not-yet-written code associated with that comment.
Any idea how this should be done? NSTextView does it properly...
Unfortunately, I am not familiar with using TSM from Cocoa... Yet, looks like there's enough evidence to at
least mark this confirmed :)
[[NSInputManager currentInputManager] handleMouseEvent:event];
at this point almost does the job. That is, an event gets forwarded, but with a wrong character index
(because -[WebHTMLView(WebNSTextInputSupport) characterIndexForPoint:] is simply not implemented
Created attachment 3862 [details]
I think that the problem with characterIndexForPoint is a separate one -
Kotoeri itself asks for the index while processing a mouse event.
Because of that, and because this patch solves the problem for my own and,
presumably, Evan's input methods, I think characterIndexForPoint should be
Comment on attachment 3862 [details]
Patch looks great.
I don't think that the check for clcking outside the marked text is good
enough. To communicate the rectangle to the input manager, it's true that we
have to use only "first rect for character range", but for us internally, we
should be able to accurately determine if a click is in the marked text or not.
So the check should not be to get a rect and check if the mouse is in the rect.
Instead we should be passing the point over to WebCore to let it determine if
the click is outside the marked text.
Formatting nitpick: This code uses formatting like "NSEvent *", but the new
code says "NSInputManager*" with no space. I'd love for this to be consistent
across the entire WebKit project, either way. Unfortunately we have a mix.
Created attachment 3879 [details]
Re-implemented using editableDOMRangeForPoint, markedTextDOMRange and
There are at least two things I don't really understand about that code path:
- What are the input managers that don't want to handle mouse events and why we
should treat them as described in the comment (AFAIK, TSM input methods always
handle mouse events).
- What editableDOMRangeForPoint actually is - I am always getting a range with
start==end from this method. Anyhow, I'm handling it as a real range, just in
Comment on attachment 3879 [details]
I'd like to see the "DOM ranges intersect" code factored into a separate
editableDOMRangeForPoint does always return a range with start == end. It uses
DOMRange just because there's no "DOMPoint".
I think this is a good change, r=me.
I'm concerned, however, that we don't have anyone testing the Japanese and
Chinese input methods. It's was quite challenging getting them working with
WebKit and Mail for Tiger, and I worry that changes in this area will result in
regressions in their behavior.
(In reply to comment #7)
> I'd like to see the "DOM ranges intersect" code factored into a separate function.
Should it be something like -(bool)intersectsDOMRange:(DOMRange*)other in DOMRange itself? I'm
wondering what would be the right way to treat end points... What seems natural to me is that the left
end point is included in the range, and the right one isn't:
[0,0) intersects [0,1)
[0,1) doesn't intersect [1,1)
[0,1) doesn't intersect [1, 2)
Does it make sense for DOMRanges?
> I worry that changes in this area will result in regressions in their behavior.
I expect quite a number of changes after this one and those Evan is proposing are finished - we need
them as a baseline for more involved ones...
The idea of including the start but not the end doesn't really make sense for a general purpose
DOMRange method. It's specific to a use of DOM ranges rather than the fundamental concept. So I
could see that in a helper function but not in a DOMRange method.
I typically think of DOM ranges as describing sets of DOM content, so if I was writing an intersection
test, then it would return false for ranges that touch only at end points and an empty range would not
even intersect with itself. That's the kind of general purpose DOM range intersection function that I
think should be around as a general purpose helper.
Another kind of helper would be a "Is this DOM point in this DOM range?" function. For that, we pass
two ranges and use the start of one range as the point. That's the helper that would be directly
applicable for this patch, and as you say we'd have to define whether "in this DOM range" includes the
endpoints or not.
I did some research, and the rule in AppKit is that selections at the end of the inline input area *do* count
as "within the marked text", so I'm going to mimic that rule.
But we have no way to test this unless we come up with an input method that does inline input but does
not handle mouse clicks. I don't know of one.
I don't know why this didn't occur to me earlier, but it's incorrect to unmark the text because of the mouse
down event. That should be done in response to a selection change, not at the time you click.
(In reply to comment #10)
> I did some research, and the rule in AppKit is that selections at the end of the inline input
> area *do* count as "within the marked text", so I'm going to mimic that rule.
FWIW, NSTextView has an obvious bug in this situation:
1) Open a new document in TextEdit
2) Type something with Kotoeri, do not close the inline input session
3) Click somewhere to the right of the marked text
Results: the caret gets positioned _before_ the last character in the inline hole.
(In reply to comment #11)
Hmm... If the selection changes because an arrow key is pressed, the inline area shouldn't get
automatically confirmed. How are mouse clicks different?
Besides, in the situation that I described in the previous comment, the caret position shouldn't change
(and it's arguable whether the inline area should be confirmed).
I completely omitted the code to unmark text when you do mouse down. The comment claimed it was a
good idea to do that, but it wasn't.
I also fixed a bug where we'd retain and not release in one of the methods.
(In reply to comment #14)
Filed the "DOM ranges intersect" suggestion as bug 5116.
Sorry for the over-retaining bug...