Bug 81042 - [WK2] WTR needs an implementation of setAsynchronousSpellCheckingEnabled
Summary: [WK2] WTR needs an implementation of setAsynchronousSpellCheckingEnabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) Unspecified
: P2 Normal
Assignee: Grzegorz Czajkowski
URL:
Keywords: InRadar, LayoutTestFailure, PlatformOnly
Depends on: 91854 108172 109577
Blocks: 108528 111295
  Show dependency treegraph
 
Reported: 2012-03-13 15:20 PDT by Jessie Berlin
Modified: 2013-03-05 05:32 PST (History)
13 users (show)

See Also:


Attachments
insufficient (11.14 KB, patch)
2012-06-04 17:02 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
adapt Tim's patch (20.35 KB, patch)
2013-02-27 03:24 PST, Grzegorz Czajkowski
enrica: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Radar WebKit Bug Importer 2012-03-13 15:23:22 PDT
<rdar://problem/11042016>
Comment 2 Jessie Berlin 2012-03-13 15:43:08 PDT
Added to the mac-wk2 Skipped list in http://trac.webkit.org/changeset/110625
Comment 3 Csaba Osztrogonác 2012-05-21 06:49:52 PDT
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
Comment 4 Tim Horton 2012-06-04 16:25:08 PDT
Just implementing setAsynchronousSpellCheckingEnabled is not sufficient to make those tests work, at least not for the Mac port.
Comment 5 Tim Horton 2012-06-04 17:02:56 PDT
Created attachment 145657 [details]
insufficient
Comment 6 Grzegorz Czajkowski 2012-07-17 08:04:19 PDT
(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(...)
Comment 7 Tim Horton 2012-07-24 17:57:43 PDT
(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.
Comment 8 Grzegorz Czajkowski 2012-08-06 06:04:25 PDT
(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.
Comment 9 Vicki Pfau 2012-10-11 13:12:33 PDT
Skipping editing/spelling/grammar-markers-hidpi.html in r131090
Comment 10 Grzegorz Czajkowski 2013-02-26 06:27:21 PST
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
Comment 11 Tim Horton 2013-02-26 10:10:05 PST
(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!
Comment 12 Grzegorz Czajkowski 2013-02-27 03:24:25 PST
Created attachment 190483 [details]
adapt Tim's patch
Comment 13 Grzegorz Czajkowski 2013-02-27 05:27:28 PST
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
Comment 14 Enrica Casucci 2013-03-04 13:17:51 PST
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.
Comment 15 Grzegorz Czajkowski 2013-03-05 05:32:26 PST
Committed r144752: <http://trac.webkit.org/changeset/144752>