Bug 69128 – Unifying spell-checking code path should be easy

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Shinya Kawanaka]

Editor class has two difference code paths for spell-checking:
(1) checkTextOfParagraph() for Snow Leopard or later
(2) checkSpellingOfString() and checkGrammarOfString() for other platforms.
These paths should be unified so that improving spell-checking code is easy.

At the first step, we should call checkTextOfParagraph() indirectly so that we can implement a function to mimic checkTextOfParagraph() in non-SL or non-Lion platform.
Also, we should introduce a flag to choose a code path which calls checkTextOfParagraph() in non-SL or non-Lion platform.

[Shinya Kawanaka]

Created attachment 109258 [details]
Patch

[WebKit Review Bot]

Attachment 109258 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/accessibility/Accessibility..." exit_code: 1

Source/WebCore/editing/TextCheckingHelper.h:98:  The parameter name "client" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/editing/Editor.cpp:1892:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/WebCore/editing/Editor.cpp:1980:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/editing/Editor.cpp:3238:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 4 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Collabora GTK+ EWS bot]

Comment on attachment 109258 [details]
Patch

Attachment 109258 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9899334

[Gyuyoung Kim]

Comment on attachment 109258 [details]
Patch

Attachment 109258 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9888737

[WebKit Review Bot]

Comment on attachment 109258 [details]
Patch

Attachment 109258 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9901303

[Early Warning System Bot]

Comment on attachment 109258 [details]
Patch

Attachment 109258 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9899335

[Shinya Kawanaka]

Created attachment 109263 [details]
Test Patch

[WebKit Review Bot]

Attachment 109263 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/accessibility/Accessibility..." exit_code: 1

Source/WebCore/editing/TextCheckingHelper.h:98:  The parameter name "client" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/editing/Editor.cpp:1892:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/WebCore/editing/Editor.cpp:1980:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/editing/Editor.cpp:3238:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 4 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 109263 [details]
Test Patch

Attachment 109263 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9898327

[Early Warning System Bot]

Comment on attachment 109263 [details]
Test Patch

Attachment 109263 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9901320

[Collabora GTK+ EWS bot]

Comment on attachment 109263 [details]
Test Patch

Attachment 109263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9900350

[Gustavo Noronha (kov)]

Comment on attachment 109263 [details]
Test Patch

Attachment 109263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9893601

[Shinya Kawanaka]

Created attachment 109274 [details]
Test Patch

[Shinya Kawanaka]

Created attachment 109280 [details]
Patch

[Shinya Kawanaka]

Created attachment 109286 [details]
Patch

[Ryosuke Niwa]

Can we split this patch into smaller pieces?

[Shinya Kawanaka]

(In reply to comment #16)
> Can we split this patch into smaller pieces?

Hmm...

Maybe we can split this patch into two patches:
1) introduces a wrapper function of TextClientChecker::checkTextOfParagraph(), and uses the wrapper instead of the original one.
2) removing static-#if statements by introducing unifiedTextCheckerEnabled.

Let me try.

[Hajime Morrita]

> Maybe we can split this patch into two patches:
> 1) introduces a wrapper function of TextClientChecker::checkTextOfParagraph(), and uses the wrapper instead of the original one.
> 2) removing static-#if statements by introducing unifiedTextCheckerEnabled.
> 
> Let me try.
Sounds nice. In that case, please create sub-bugs which depends on this bug.

[Hajime Morrita]

Closing this since dependent bugs are closed.