Bug 81042 – [WK2] WTR needs an implementation of setAsynchronousSpellCheckingEnabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jessie Berlin]

The lack of it is causing failures on the Lion Intel Debug WebKit 2 bots:

http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r110608%20(4861)/editing/spelling/grammar-paste-pretty-diff.html
http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r110608%20(4861)/editing/spelling/spellcheck-async-mutation-pretty-diff.html
http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r110608%20(4861)/editing/spelling/spellcheck-async-pretty-diff.html
http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r110608%20(4861)/editing/spelling/spellcheck-queue-pretty-diff.html
http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r110608%20(4861)/editing/spelling/spellcheck-sequencenum-pretty-diff.html

They should be added to the mac-wk2 skipped list for now.

[Radar WebKit Bug Importer]

<rdar://problem/11042016>

[Jessie Berlin]

Added to the mac-wk2 Skipped list in http://trac.webkit.org/changeset/110625

[Csaba Osztrogonác]

It isn't mac-wk2 spefific bug, but general wk2 bug, so I moved tests
to wk2/Skipped list and added one more test. - http://trac.webkit.org/changeset/117776

[Tim Horton]

Just implementing setAsynchronousSpellCheckingEnabled is not sufficient to make those tests work, at least not for the Mac port.

[Tim Horton]

Created attachment 145657 [details]
insufficient

[Grzegorz Czajkowski]

(In reply to comment #5)
> Created an attachment (id=145657) [details]
> insufficient

WebKit2-EFL needs this patch as we've almost done spellchecker feature and we would like to perform the tests from editing/spelling directory, btw:

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2010
> +    settings->setFullScreenEnabled(store.getBoolValueForKey(WebPreferencesKey::asynchronousSpellCheckingEnabledKey()));

should be settings->setAsynchronousSpellCheckingEnabled(...)

[Tim Horton]

(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=145657) [details] [details]
> > insufficient
> 
> WebKit2-EFL needs this patch as we've almost done spellchecker feature and we would like to perform the tests from editing/spelling directory, btw:
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=145657&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2010
> > +    settings->setFullScreenEnabled(store.getBoolValueForKey(WebPreferencesKey::asynchronousSpellCheckingEnabledKey()));
> 
> should be settings->setAsynchronousSpellCheckingEnabled(...)

Erg, good catch. That explains why it didn't work! You're welcome to pick up the patch and fix that and see if it works.

[Grzegorz Czajkowski]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Created an attachment (id=145657) [details] [details] [details]
> > > insufficient
> > 
> > WebKit2-EFL needs this patch as we've almost done spellchecker feature and we would like to perform the tests from editing/spelling directory, btw:
> > 
> > View in context: https://bugs.webkit.org/attachment.cgi?id=145657&action=review
> > 
> > > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2010
> > > +    settings->setFullScreenEnabled(store.getBoolValueForKey(WebPreferencesKey::asynchronousSpellCheckingEnabledKey()));
> > 
> > should be settings->setAsynchronousSpellCheckingEnabled(...)
> 
> Erg, good catch. That explains why it didn't work! You're welcome to pick up the patch and fix that and see if it works.

Ok thanks. I will submit this patch with the fix and ChangeLog entries.

[Vicki Pfau]

Skipping editing/spelling/grammar-markers-hidpi.html in r131090

[Grzegorz Czajkowski]

Hi Tim,

I am adapting your patch to the trunk. The required patches (bug 108172 and bug 109577) are likely to land.
I would like to expose asynchronous spell checking setting in this patch as it may be useful for WK2 ports that are going to enable it. In my opinion it doesn't make sense to implement it as private and then make another commit to make it public.
This patch will contain gardening of asynchronous spell checking tests. I am planing to skip them for GtK, Mac, Qt unless they implement TextChecker::requestCheckingOfString() method.
Is it ok for you if I add myself as co-author of the patch?
Thanks

[Tim Horton]

(In reply to comment #10)
> Hi Tim,
> 
> I am adapting your patch to the trunk. The required patches (bug 108172 and bug 109577) are likely to land.
> I would like to expose asynchronous spell checking setting in this patch as it may be useful for WK2 ports that are going to enable it. In my opinion it doesn't make sense to implement it as private and then make another commit to make it public.
> This patch will contain gardening of asynchronous spell checking tests. I am planing to skip them for GtK, Mac, Qt unless they implement TextChecker::requestCheckingOfString() method.
> Is it ok for you if I add myself as co-author of the patch?

Go for it!

> Thanks

Thanks for picking this up!

[Grzegorz Czajkowski]

Created attachment 190483 [details]
adapt Tim's patch

[Grzegorz Czajkowski]

FYI, with this patch (and its dependencies bug 109577 and 108172) the below asynchronous spell checking tests are passing for EFL:

editing/spelling/spellcheck-async-remove-frame.html
editing/spelling/spellcheck-async.html
editing/spelling/spellcheck-sequencenum.html

The remaining asynchronous tests:
editing/spelling/spellcheck-async-mutation.html
editing/spelling/spellcheck-queue.html
editing/spelling/spellcheck-paste-disabled.html
editing/spelling/spellcheck-paste.html
editing/spelling/grammar-markers.html
editing/spelling/grammar-markers-hidpi.html
editing/spelling/grammar-paste.html
editing/spelling/spelling-marker-description.html

are failing for EFL due to not implemented features (pasteboard, grammar checking and text replacement). More details in TestExpectations file.
I was trying to do some tricks to pass the failing tests like replacing missing Paste command with InsertText and as a result, they start passing.

Recently, the bunch of spell checking tests have been landed (bug 108405). Those require asynchronous spell checking and smartInsertDelete feature (bug 107840).

I'd appreciate if wk2's owners could review the mentioned patches to support asynchronous spell checking in WK2 (bug 109577, bug 108405, bug 108172 and bug 81042)
Thanks

[Enrica Casucci]

Comment on attachment 190483 [details]
adapt Tim's patch

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

Looks good to me. Please remove all the unnecessary lines from the ChangeLog.

> Source/WebKit2/ChangeLog:15
> +        (WebKit):

Remove.

> Source/WebKit2/ChangeLog:25
> +        (WebKit):

Remove.

> Source/WebKit2/ChangeLog:27
> +        (InjectedBundle):

Remove.

> Tools/ChangeLog:15
> +        (WTR):

Remove.

> Tools/ChangeLog:17
> +        (TestRunner):

Ditto.

[Grzegorz Czajkowski]

Committed r144752: <http://trac.webkit.org/changeset/144752>