[webkit-reviews] review denied: [Bug 14710] DumpRenderTree needs to be able to test Input Method behaviour : [Attachment 15617] Woo, an ability to test input method behaviour!!!!!

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 14710: DumpRenderTree needs to be able to test Input Method behaviour
http://bugs.webkit.org/show_bug.cgi?id=14710

Attachment 15617: Woo, an ability to test input method behaviour!!!!!
http://bugs.webkit.org/attachment.cgi?id=15617&action=edit

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+	 * ChangeLog:

This needn't be in the ChangeLog.

+	 (-[InputMethodController setIMEFunction:]):

I'd call this setIMEventHandler. Generally, it would be nice not to say "IME"
in Mac code.

Index: DumpRenderTree/InputMethodController.h

Would it make sense to implement this within TextInputContoller, which already
has NSTextInput methods? We seem to get quite a bit of code duplication.

+    WebScriptObject *imeFunction;
+    WebHTMLView *view;

  Coding style: please prefix Objective-C instance variables with "_".

+    WebScriptObject *modifiers = [imeFunction evaluateWebScript:@"new
Array();"];

  I think you can work with an NSArray, and converted to WebScriptObject later
- the current code doesn't look nice. Mark had more to say about that on IRC.

  r- mostly because of code duplication.