[Webkit-unassigned] [Bug 50788] Implement IME support for Mac in WebKit2

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50788


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #76126|review?                     |review+
               Flag|                            |




--- Comment #2 from Alexey Proskuryakov <ap at webkit.org>  2010-12-09 16:40:46 PST ---
(From update of attachment 76126)
View in context: https://bugs.webkit.org/attachment.cgi?id=76126&action=review

> WebCore/ChangeLog:11
> +        (WebCore::KeypressCommand::KeypressCommand): Removed ASSERT in constructor,
> +        since it is now used for more than one command.

This patch expands the set of messages encoded in KeypressCommand, changing it into a strange "Objective-C invocation with one string argument". Losing the semantic meaning of the class is not great.

On the other hand, I can't even easily describe its previous semantic meaning anyway. Something that can be sent when handling a key, which is any message without arguments or insertText, but nothing else.

> WebKit2/ChangeLog:11
> +        to the WebProcess. These calls all have a timeout of 1 second.

Unfortunately, attributedSubstringFromRange can legitimately take a lot of time if the client asks for the whole document. AppKit has been known to do that in some cases.

So, please test behavior with large documents, especially when adding attributedSubstringFromRange.

> WebKit2/ChangeLog:44
> +        (-[WKView conversationIdentifier]): Added.

Actually, moved.

> WebKit2/UIProcess/API/mac/PageClientImpl.mm:231
> +    commandsList = [m_wkView _interceptKeyEvent:event.nativeEvent()
> +                                 hasComposition:hasComposition 
> +                                 selectionStart:selectionStart
> +                                   selectionEnd:selectionEnd
> +                                     underlines:underlines];

Can these be combined in a single argument with a helpful name, something like StateForTextInput? The method doesn't really need any of these, so passing them into it can confuse the reader.

> WebKit2/UIProcess/API/mac/WKView.mm:476
> +    _data-> _hasMarkedText = hasComposition;

Extra space here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.