Bug 117585 - Context menu grammar checking items are available when GRAMMAR_CHECKING macro is off
Summary: Context menu grammar checking items are available when GRAMMAR_CHECKING macro...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Grzegorz Czajkowski
URL:
Keywords:
Depends on: 119086
Blocks: 117692
  Show dependency treegraph
 
Reported: 2013-06-13 01:43 PDT by Grzegorz Czajkowski
Modified: 2013-07-25 06:47 PDT (History)
6 users (show)

See Also:


Attachments
guard grammar checking in context menu code (33.99 KB, patch)
2013-06-13 01:52 PDT, Grzegorz Czajkowski
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2013-06-13 01:43:50 PDT
"Check Grammar With Spelling" and "Ignore Grammar" shouldn't be populated in the context menu when GRAMMAR_CHECKING is off.

"Spelling and Grammar" should be replaced with "Spelling" as the sub menu title and "Show/Hide Spelling and Grammar" with "Show/Hide Spelling" when GRAMMAR_CHECKING is off.
Comment 1 Grzegorz Czajkowski 2013-06-13 01:52:41 PDT
Created attachment 204560 [details]
guard grammar checking in context menu code
Comment 2 Anders Carlsson 2013-06-14 06:17:24 PDT
Comment on attachment 204560 [details]
guard grammar checking in context menu code

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

> Source/WebCore/platform/LocalizedStrings.cpp:304
>      if (show)
> +#if USE(GRAMMAR_CHECKING)
>          return WEB_UI_STRING("Show Spelling and Grammar", "menu item title");
>      return WEB_UI_STRING("Hide Spelling and Grammar", "menu item title");
> +#else
> +        return WEB_UI_STRING("Show Spelling", "menu item title");
> +    return WEB_UI_STRING("Hide Spelling", "menu item title");
> +#endif

I'd rather if (show) was duplicated here, once in the #if block and once in the #else block.
Comment 3 Grzegorz Czajkowski 2013-06-16 23:50:08 PDT
(In reply to comment #2)
> (From update of attachment 204560 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204560&action=review
> 
> > Source/WebCore/platform/LocalizedStrings.cpp:304
> >      if (show)
> > +#if USE(GRAMMAR_CHECKING)
> >          return WEB_UI_STRING("Show Spelling and Grammar", "menu item title");
> >      return WEB_UI_STRING("Hide Spelling and Grammar", "menu item title");
> > +#else
> > +        return WEB_UI_STRING("Show Spelling", "menu item title");
> > +    return WEB_UI_STRING("Hide Spelling", "menu item title");
> > +#endif
> 
> I'd rather if (show) was duplicated here, once in the #if block and once in the #else block.

OK, fixed.
Comment 4 Grzegorz Czajkowski 2013-06-17 00:29:40 PDT
Committed r151632: <http://trac.webkit.org/changeset/151632>
Comment 5 Gábor Ábrahám 2013-06-17 02:19:41 PDT
In Qt Debug build bots both x32 and x64 there is a compilation error :

stdio log: 
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp: In member function 'void WebCore::Editor::advanceToNextMisspelling(bool)':
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp:1771:12: error: 'WTF_USE_GRAMMAR_CHECKING' was not declared in this scope
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp: In member function 'void WebCore::Editor::markMisspellingsOrBadGrammar(const WebCore::VisibleSelection&, bool, WTF::RefPtr<WebCore::Range>&)':
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp:2067:12: error: 'WTF_USE_GRAMMAR_CHECKING' was not declared in this scope
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp: In member function 'void WebCore::Editor::markBadGrammar(const WebCore::VisibleSelection&)':
/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/editing/Editor.cpp:2095:8: error: 'WTF_USE_GRAMMAR_CHECKING' was not declared in this scope

Link: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Debug/builds/29347

Could you check it please?
Comment 6 Zoltan Arvai 2013-06-17 02:28:27 PDT
Similar build failure on GTK Linux 64-bit Debug WK1
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20WK1/builds/2607

also problems on Apple Win Debug:
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/67751
Comment 7 Grzegorz Czajkowski 2013-06-17 06:32:57 PDT
Reverted r151632 for reason:

Debug build error ASSERT(WTF_USE_GRAMMAR_CHECKING) for non MAC platforms

Committed r151638: <http://trac.webkit.org/changeset/151638>