Bug 146904

Summary: [GTK] Build error with -DENABLE_SPELLCHECK=OFF
Product: WebKit Reporter: nick <unixman>
Component: WebKitGTKAssignee: Carlos Alberto Lopez Perez <clopez>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, clopez, mario, mrobinson
Priority: P2    
Version: 420+   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch cgarcia: review+

Description nick 2015-07-13 10:35:57 PDT
gtk port can not built when -DENABLE_SPELLCHECK=OFF. it complains

error: TextCheckerEnchant was not defined

But cmake allows turn it OFF when generating makefile.
Comment 1 Carlos Alberto Lopez Perez 2015-09-22 10:20:42 PDT
I ACK this bug. When building WebKitGTK+ with -DENABLE_SPELLCHECK=OFF it fails to build with:


../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:43:17: error: no type named 'TextCheckerEnchant' in namespace 'WebCore'; did you mean 'TextCheckerClient'?
static WebCore::TextCheckerEnchant& enchantTextChecker()
       ~~~~~~~~~^~~~~~~~~~~~~~~~~~
                TextCheckerClient
../../Source/WebCore/page/EditorClient.h:63:7: note: 'TextCheckerClient' declared here
class TextCheckerClient;
      ^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:45:36: error: no member named 'TextCheckerEnchant' in namespace 'WebCore'
    static NeverDestroyed<WebCore::TextCheckerEnchant> checker;
                          ~~~~~~~~~^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:116:79: error: cannot initialize a parameter of type 'int *' with an lvalue of type 'int32_t' (aka 'int')
    enchantTextChecker().checkSpellingOfString(text.toStringWithoutCopying(), misspellingLocation, misspellingLength);
                                                                              ^~~~~~~~~~~~~~~~~~~
../../Source/WebCore/platform/text/TextCheckerClient.h:47:57: note: passing argument to parameter 'misspellingLocation' here
    virtual void checkSpellingOfString(StringView, int* misspellingLocation, int* misspellingLength) = 0;
                                                        ^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:142:58: error: too few arguments to function call, expected 3, have 1
    guesses = enchantTextChecker().getGuessesForWord(word);
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~     ^
../../Source/WebCore/platform/text/TextCheckerClient.h:58:5: note: 'getGuessesForWord' declared here
    virtual void getGuessesForWord(const String& word, const String& context, Vector<String>& guesses) = 0;
    ^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:152:26: error: no member named 'ignoreWord' in 'WebCore::TextCheckerClient'
    enchantTextChecker().ignoreWord(word);
    ~~~~~~~~~~~~~~~~~~~~ ^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:233:26: error: no member named 'updateSpellCheckingLanguages' in 'WebCore::TextCheckerClient'
    enchantTextChecker().updateSpellCheckingLanguages(languages);
    ~~~~~~~~~~~~~~~~~~~~ ^
../../Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:238:33: error: no member named 'loadedSpellCheckingLanguages' in 'WebCore::TextCheckerClient'
    return enchantTextChecker().loadedSpellCheckingLanguages();
           ~~~~~~~~~~~~~~~~~~~~ ^
Comment 2 Carlos Alberto Lopez Perez 2015-09-22 10:37:49 PDT
Created attachment 261749 [details]
Patch
Comment 3 Carlos Alberto Lopez Perez 2015-09-29 02:39:36 PDT
(In reply to comment #2)
> Created attachment 261749 [details]
> Patch

ping reviewers?
Comment 4 Carlos Garcia Campos 2015-09-29 03:54:12 PDT
Comment on attachment 261749 [details]
Patch

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

> Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:37
> +#if ENABLE(SPELLCHECK)
> +#include "TextBreakIterator.h"
>  #include "WebProcessPool.h"
>  #include "WebTextChecker.h"
> +#endif

Why do we need to protect these? WebTextChecker.h could even be removed, I would say. And the others are not specific to spellchecker. Headers should have their own guards if needed, like TextCheckerEnchant.h
Comment 5 Carlos Alberto Lopez Perez 2015-09-30 05:43:03 PDT
(In reply to comment #4)
> Comment on attachment 261749 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261749&action=review
> 
> > Source/WebKit2/UIProcess/gtk/TextCheckerGtk.cpp:37
> > +#if ENABLE(SPELLCHECK)
> > +#include "TextBreakIterator.h"
> >  #include "WebProcessPool.h"
> >  #include "WebTextChecker.h"
> > +#endif
> 
> Why do we need to protect these? WebTextChecker.h could even be removed, I
> would say. And the others are not specific to spellchecker. Headers should
> have their own guards if needed, like TextCheckerEnchant.h

Not other reason that avoiding to include not needed files when the feature is not enabled.

I will remove WebTextChecker.h form the list and remove the ifdef for the headers.
Comment 6 Carlos Alberto Lopez Perez 2015-09-30 05:55:49 PDT
Committed r190345: <http://trac.webkit.org/changeset/190345>