Bug 73616

Summary:

Asynchronous path and synchronous path of SpellChecker should share the code to mark misspellings

Product:

WebKit

Reporter:

Shinya Kawanaka <shinyak>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, morrita, shinyak, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

73628, 73889, 74033, 74035

Bug Blocks:

56368

Attachments:

Description	Flags
Patch	none
Patch	morrita: review-
Patch	none
Patch	none

Shinya Kawanaka

Reported 2011-12-01 20:30:36 PST

Currently the unified spellchecker and un-unified spellchecker do not share the code to mark misspellings. They should be merged.

Attachments
Patch (27.40 KB, patch) 2011-12-06 03:43 PST, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (27.21 KB, patch) 2011-12-07 02:59 PST, Shinya Kawanaka	morrita: review-	Details Formatted Diff Diff
Patch (4.46 KB, patch) 2011-12-08 22:44 PST, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (8.08 KB, patch) 2011-12-09 04:16 PST, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Shinya Kawanaka

Comment 1 2011-12-06 03:43:27 PST

Created attachment 118019 [details] Patch

Shinya Kawanaka

Comment 2 2011-12-07 02:59:56 PST

Created attachment 118196 [details] Patch

Hajime Morrita

Comment 3 2011-12-07 03:30:47 PST

Comment on attachment 118196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118196&action=review Could you split the patch into some trivial part (like renaming, exporting an internal class, etc.) and possibly-observable change like make marking code shared? > Source/WebCore/ChangeLog:6 > + Need a short description and bug URL (OOPS!) You don't need this. > Source/WebCore/editing/SpellChecker.h:80 > + class InternalSpellCheckRequest; This doesn't need to be a nested class.

Shinya Kawanaka

Comment 4 2011-12-08 22:44:48 PST

Created attachment 118534 [details] Patch

WebKit Review Bot

Comment 5 2011-12-08 22:48:47 PST

Comment on attachment 118534 [details] Patch Attachment 118534 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10780559

Hajime Morrita

Comment 6 2011-12-09 01:00:38 PST

Comment on attachment 118534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118534&action=review > Source/WebCore/editing/SpellChecker.cpp:175 > + m_frame->editor()->markAndReplaceFor(m_processingRequest->mask(), results, m_processingRequest->checkingRange(), m_processingRequest->paragraphRange()); Could you pass SpellCheckRequest directly markAndReplaceFor() , instead of passing independent parameters.

Shinya Kawanaka

Comment 7 2011-12-09 04:16:04 PST

Created attachment 118558 [details] Patch

Hajime Morrita

Comment 8 2011-12-11 17:51:50 PST

Comment on attachment 118558 [details] Patch Looks OK. Thanks for doing this!

Hajime Morrita

Comment 9 2011-12-11 17:53:02 PST

Once this landed, could you write a test for Bug 56368?

WebKit Review Bot

Comment 10 2011-12-11 19:12:22 PST

Comment on attachment 118558 [details] Patch Clearing flags on attachment: 118558 Committed r102553: <http://trac.webkit.org/changeset/102553>

WebKit Review Bot

Comment 11 2011-12-11 19:12:31 PST

All reviewed patches have been landed. Closing bug.

Shinya Kawanaka

Comment 12 2011-12-11 20:41:59 PST

(In reply to comment #9) > Once this landed, could you write a test for Bug 56368? Yes.

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