RESOLVED FIXED 73616
Asynchronous path and synchronous path of SpellChecker should share the code to mark misspellings 
https://bugs.webkit.org/show_bug.cgi?id=73616
Summary Asynchronous path and synchronous path of SpellChecker should share the code ...
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
DetailsFormatted DiffDiff
Patch (27.21 KB, patch)
2011-12-07 02:59 PST, Shinya Kawanaka 		morrita: review-
DetailsFormatted DiffDiff
Patch (4.46 KB, patch)
2011-12-08 22:44 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (8.08 KB, patch)
2011-12-09 04:16 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
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.