Bug 118392

Summary: [EFL][WK2] Implement unit test callbacks: onWordLearn and onWordIgnore
Product: WebKit Reporter: Krzysztof Wolanski <k.wolanski>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, gyuyoung.kim, lucas.de.marchi, rakuco
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch
none
apply suggestions
none
apply suggestions
cdumez: review+
apply suggestions none

Description Krzysztof Wolanski 2013-07-04 06:47:11 PDT 
Simulate behavior of Learn and Ignore Word in the text field.
Comment 1 Krzysztof Wolanski 2013-07-04 06:50:14 PDT 
Created attachment 206084 [details]
proposed patch
Comment 2 Chris Dumez 2013-07-04 07:51:33 PDT 
Comment on attachment 206084 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206084&action=review

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:807
> +    knownWord = "";

knownWord = String()?

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:839
> +    knownWord = "";

knownWord = String()?
Comment 3 Mikhail Pozdnyakov 2013-07-04 07:58:36 PDT 
Comment on attachment 206084 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206084&action=review

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:55
> +String knownWord;

why not static?

>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:807
>> +    knownWord = "";
> 
> knownWord = String()?

emptyString() ?
Comment 4 Krzysztof Wolanski 2013-07-04 08:02:36 PDT 
Created attachment 206088 [details]
apply suggestions
Comment 5 Chris Dumez 2013-07-04 08:03:51 PDT 
> >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:807
> >> +    knownWord = "";
> > 
> > knownWord = String()?
> 
> emptyString() ?

Right, emptyString() is better indeed.
Comment 6 Chris Dumez 2013-07-04 08:04:26 PDT 
Why no review flag?
Comment 7 Krzysztof Wolanski 2013-07-04 08:08:06 PDT 
Created attachment 206090 [details]
apply suggestions
Comment 8 Chris Dumez 2013-07-04 08:14:20 PDT 
Comment on attachment 206090 [details]
apply suggestions

Ok, r=me but please let Grzegorz have a look before landing.
Comment 9 Grzegorz Czajkowski 2013-07-05 00:07:46 PDT 
Comment on attachment 206090 [details]
apply suggestions

View in context: https://bugs.webkit.org/attachment.cgi?id=206090&action=review

I'd love to see it in the trunk. Added one comments regarding to the spellchecking and one nit.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:188
> +    *misspellingLength = static_cast<int32_t>(knownWord.length());

As I understand correctly, this callback should treat known words as spelled correctly. Therefore, its length should be set to 0 to notify WebCore to don't create spelling marker for it. Additionally, the location should be set to -1 according to the doc.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:813
> +    mouseDoubleClick(10, 20, 1 /* Left button */);

Can we skip left button value and this comment here? mouseDoubleClick passes left button implicitly.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:845
> +    mouseDoubleClick(10, 20, 1 /* Left button */);

Ditto.
Comment 10 Krzysztof Wolanski 2013-07-05 01:45:37 PDT 
Created attachment 206127 [details]
apply suggestions
Comment 11 Grzegorz Czajkowski 2013-07-05 02:48:25 PDT 
Comment on attachment 206127 [details]
apply suggestions

Thanks
Comment 12 WebKit Commit Bot 2013-07-05 03:29:59 PDT 
Comment on attachment 206127 [details]
apply suggestions

Clearing flags on attachment: 206127

Committed r152414: <http://trac.webkit.org/changeset/152414>
Comment 13 WebKit Commit Bot 2013-07-05 03:30:02 PDT 
All reviewed patches have been landed.  Closing bug.