Bug 69128

Summary:

Unifying spell-checking code path should be easy

Product:

WebKit

Reporter:

Shinya Kawanaka <shinyak>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, dglazkov, gustavo.noronha, gustavo, morrita, rniwa, shinyak, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

69241, 69242

Bug Blocks:

65849

Attachments:

Description	Flags
Patch	gustavo.noronha: commit-queue-
Test Patch	gyuyoung.kim: commit-queue-
Test Patch	none
Patch	none
Patch	none

Shinya Kawanaka

Reported 2011-09-29 23:17:14 PDT

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.

Attachments
Patch (35.20 KB, patch) 2011-09-30 00:00 PDT, Shinya Kawanaka	gustavo.noronha: commit-queue-	Details Formatted Diff Diff
Test Patch (35.34 KB, patch) 2011-09-30 01:20 PDT, Shinya Kawanaka	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
Test Patch (42.65 KB, patch) 2011-09-30 03:15 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (42.65 KB, patch) 2011-09-30 04:16 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (47.01 KB, patch) 2011-09-30 05:23 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Shinya Kawanaka

Comment 1 2011-09-30 00:00:14 PDT

Created attachment 109258 [details] Patch

WebKit Review Bot

Comment 2 2011-09-30 00:02:41 PDT

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 3 2011-09-30 00:08:22 PDT

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 4 2011-09-30 00:09:25 PDT

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 5 2011-09-30 00:12:36 PDT

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 6 2011-09-30 00:14:00 PDT

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

Shinya Kawanaka

Comment 7 2011-09-30 01:20:11 PDT

Created attachment 109263 [details] Test Patch

WebKit Review Bot

Comment 8 2011-09-30 01:23:32 PDT

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 9 2011-09-30 01:31:23 PDT

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 10 2011-09-30 01:32:07 PDT

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 11 2011-09-30 02:24:50 PDT

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 12 2011-09-30 02:48:32 PDT

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

Comment 13 2011-09-30 03:15:51 PDT

Created attachment 109274 [details] Test Patch

Shinya Kawanaka

Comment 14 2011-09-30 04:16:15 PDT

Created attachment 109280 [details] Patch

Shinya Kawanaka

Comment 15 2011-09-30 05:23:46 PDT

Created attachment 109286 [details] Patch

Ryosuke Niwa

Comment 16 2011-10-02 21:15:36 PDT

Can we split this patch into smaller pieces?

Shinya Kawanaka

Comment 17 2011-10-02 21:35:30 PDT

(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

Comment 18 2011-10-02 22:28:28 PDT

> 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

Comment 19 2011-11-10 17:19:58 PST

Closing this since dependent bugs are closed.

Note You need to log in before you can comment on or make changes to this bug.