Bug 62970 – Spellcheck API should be build-able.

• Component: HTML Editing
• Status: RESOLVED INVALID

[Hajime Morrita]

Currently Spellcheck API (ENABLE(SPELLCHECK_API)) is only buildable on Chromium. it should be available on other platforms.

[Hajime Morrita]

Created attachment 97921 [details]
Patch

[Kent Tamura]

Comment on attachment 97921 [details]
Patch

Looks ok.

[Hajime Morrita]

Comment on attachment 97921 [details]
Patch

Thanks for taking look. asking cq...

[WebKit Review Bot]

Comment on attachment 97921 [details]
Patch

Rejecting attachment 97921 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 1

Last 500 characters of output:
008c3851e66e42c39bfc36c52a04b2f3a3ec6b99
r89340 = 94a6fc82c3820f3e38fa54695b630d328866e782
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/8908813

[Kent Tamura]

Comment on attachment 97921 [details]
Patch

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

> ChangeLog:4
> [phone]  MORITA Hajime <[e-mail]>
> +
> +        Spellcheck API should be build-able.
> +        https://bugs.webkit.org/show_bug.cgi?id=62970

No "Reviewed by" line

[Hajime Morrita]

Committed r89401: <http://trac.webkit.org/changeset/89401>

[Hajime Morrita]

Reverted r89401 for reason:

Breaks mac build and mistakenly enables the spellcheck API

Committed r89406: <http://trac.webkit.org/changeset/89406>

[Hajime Morrita]

Created attachment 98126 [details]
Patch

[Hajime Morrita]

The reverted patch has several problems that ews didn'T catch:
- enabled SPELLCHECK_API on mac and windows
- DerivedSources.cpp didn't contain generated files.
I addressed that issue in new patch.

I'll land this if following thread goes well.
https://lists.webkit.org/pipermail/webkit-dev/2011-June/017240.html

[Kent Tamura]

(In reply to comment #9)

> - DerivedSources.cpp didn't contain generated files.

Does it solve a build issue in a case that SPELLCHECK_API is enabled?

[Kent Tamura]

Comment on attachment 98126 [details]
Patch

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

> Source/WebCore/GNUmakefile.am:166
> +FEATURE_DEFINES += ENABLE_SPELLCHECK_API=1
> +webcore_cppflags += -DENABLE_SPELLCHECK_API=1

Do you enable SPELLCHECK_API on GTK by default?

> Source/WebCore/features.pri:79
> +!contains(DEFINES, ENABLE_SPELLCHECK_API=.): DEFINES += ENABLE_SPELLCHECK_API=1

Do you enable SPELLCHECK_API on Qt by default?

[Hajime Morrita]

Created attachment 98138 [details]
Patch

[Hajime Morrita]

Hi Kent-san, thank you for reviewing again!

(In reply to comment #11)
> (From update of attachment 98126 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98126&action=review
> 
> > Source/WebCore/GNUmakefile.am:166
> > +FEATURE_DEFINES += ENABLE_SPELLCHECK_API=1
> > +webcore_cppflags += -DENABLE_SPELLCHECK_API=1
> 
> Do you enable SPELLCHECK_API on GTK by default?
No. In my understanding, this is used for EABlE_SPELLCHECK_API is given by autoconf.
Isn't it correct?

> 
> > Source/WebCore/features.pri:79
> > +!contains(DEFINES, ENABLE_SPELLCHECK_API=.): DEFINES += ENABLE_SPELLCHECK_API=1
> 
> Do you enable SPELLCHECK_API on Qt by default?
No, I changed the default value to 0

[Kent Tamura]

Comment on attachment 98126 [details]
Patch

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

>>> Source/WebCore/GNUmakefile.am:166
>>> +webcore_cppflags += -DENABLE_SPELLCHECK_API=1
>> 
>> Do you enable SPELLCHECK_API on GTK by default?
> 
> No. In my understanding, this is used for EABlE_SPELLCHECK_API is given by autoconf.
> Isn't it correct?

Ah you're right.  I overlooked "if ENABLE_SPELLCHECK_API"

[Kent Tamura]

Comment on attachment 98138 [details]
Patch

Clearing the review flag because the spellcheck API implementation has been reverted.

[Hajime Morrita]

Closing the bug. Future Spellcheck API will have different set of changes to land and keeping this would be just confusing.