Bug 14710 - DumpRenderTree needs to be able to test Input Method behaviour
Summary: DumpRenderTree needs to be able to test Input Method behaviour
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-22 03:35 PDT by Oliver Hunt
Modified: 2007-07-22 14:14 PDT (History)
1 user (show)

See Also:


Attachments
Woo, an ability to test input method behaviour!!!!! (17.74 KB, patch)
2007-07-22 03:45 PDT, Oliver Hunt
ap: review-
Details | Formatted Diff | Diff
Mark 2, this time without repeating a lot of code i wish i had realised was there (15.30 KB, patch)
2007-07-22 08:15 PDT, Oliver Hunt
darin: review+
Details | Formatted Diff | Diff
Move current IM tests to editing/input/mac, update skip lists (7.36 KB, patch)
2007-07-22 14:00 PDT, Oliver Hunt
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2007-07-22 03:35:15 PDT
Due to the continual breakage and unbreakage of Input Methods I have been going mad.

We really need some ability to ensure we don't continually regress Input Method behaviour, eg. DRT needs to support IM tests.
Comment 1 Oliver Hunt 2007-07-22 03:45:03 PDT
Created attachment 15617 [details]
Woo, an ability to test input method behaviour!!!!!
Comment 2 Alexey Proskuryakov 2007-07-22 04:19:49 PDT
Comment on attachment 15617 [details]
Woo, an ability to test input method behaviour!!!!!

+        * 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.
Comment 3 Oliver Hunt 2007-07-22 08:15:48 PDT
Created attachment 15622 [details]
Mark 2, this time without repeating a lot of code i wish i had realised was there

Here we go again
Comment 4 Oliver Hunt 2007-07-22 08:19:24 PDT
Alas I can't use NSDictionary as it doesn't get mapped into a usable JS object, so the Event structure is still a damnable WebScriptObject.

I'm thinking the best approach the the IMs is to build up <IMengine.js> for whatever engines we have bugs against, and as time goes by incorporate more of each IMs behaviour into the scripts.
Comment 5 Darin Adler 2007-07-22 08:47:08 PDT
Comment on attachment 15622 [details]
Mark 2, this time without repeating a lot of code i wish i had realised was there

Seems like a great start for testing this sort of thing. My one suggestion would be to have more of this done with an eye toward future platform-independence, for example leaving the "NS" out of the string constants for our JavaScript API.

r=me
Comment 6 Alexey Proskuryakov 2007-07-22 12:45:11 PDT
A minor comment: earlier NSTextInput tests are in editing/input, I think the test from this patch can go there, too.
Comment 7 Oliver Hunt 2007-07-22 12:51:05 PDT
Landed in r r24517
Without Alexeys test location, as his comment came though just as i committed, is it really worth moving them? :-/

Comment 8 Alexey Proskuryakov 2007-07-22 13:05:45 PDT
Well, there doesn't seem to be any reason to have two directories for these tests :(
Comment 9 Oliver Hunt 2007-07-22 13:07:37 PDT
I have a reason: I can just add the entire inputmethods directory to the windows skiplist

In future we may need to have intputmethods/mac and inputmethods/win in which case keeping them isolated will be beneficial
Comment 10 Alexey Proskuryakov 2007-07-22 13:12:04 PDT
Maybe editing/input/mac will do then?
Comment 11 Oliver Hunt 2007-07-22 13:31:31 PDT
I would prefer to keep them completely seperate from the other input methods
...
how about
events/input (as you say)
and events/input/inputengines/<engine, eg. kotoeri>/<platform>

If we could come up with someway of getting the right engine for the platform it would be great
and then just have a generic test with a non-generic engine implementation...
Comment 12 Alexey Proskuryakov 2007-07-22 13:45:34 PDT
(In reply to comment #11)
> events/input (as you say)

  More precisely, editing/input

> and events/input/inputengines/<engine, eg. kotoeri>/<platform>

  I'm not quite convinced that most of these tests will be engine-based, but I'm OK with trying such a structure.
Comment 13 Oliver Hunt 2007-07-22 13:53:26 PDT
Well i'm assuming that we'll be making tests to ensure we don't regress various IM engines, so engines would contain (for example) kotoeri.js, and hangul.js, because otherwise whenever you want to test something you putting quite a bit of effort just to ensure the rest of the IM behaviour is correct.

For now editing/input/mac would do, but i'm trying to think of the future
Comment 14 Oliver Hunt 2007-07-22 14:00:20 PDT
Created attachment 15627 [details]
Move current IM tests to editing/input/mac, update skip lists
Comment 15 Oliver Hunt 2007-07-22 14:14:22 PDT
Moved tests in r24522