Bug 73628

Summary: Refactoring: Editor::markAllMisspellingsAndBadGrammarInRanges should be refactored.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, morrita, shinyak, vsevik, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 73902    
Bug Blocks: 73616    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Shinya Kawanaka
Reported 2011-12-01 23:04:09 PST
Editor::markAllMisspellingsAndBadGrammarInRanges() should be refactored to unify the paths of Unified SpellChecker and Un-unified SpellChecker for adding markers.
Attachments
Patch (8.85 KB, patch)
2011-12-05 01:32 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (10.51 KB, patch)
2011-12-05 15:47 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (10.47 KB, patch)
2011-12-06 18:02 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2011-12-01 23:27:00 PST
(In reply to comment #0) > Editor::markAllMisspellingsAndBadGrammarInRanges() should be refactored > to unify the paths of Unified SpellChecker and Un-unified SpellChecker for adding markers. Also, SpellChecker::requestCheckingFor should be refactored.
Shinya Kawanaka
Comment 2 2011-12-05 01:32:04 PST
Created attachment 117855 [details] Patch
WebKit Review Bot
Comment 3 2011-12-05 02:19:47 PST
Comment on attachment 117855 [details] Patch Attachment 117855 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10734618 New failing tests: svg/custom/linking-uri-01-b.svg
Hajime Morrita
Comment 4 2011-12-05 03:24:15 PST
Comment on attachment 117855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117855&action=review Basically looks nice. There are only a few nits. > Source/WebCore/editing/Editor.cpp:2045 > + ExceptionCode ec = 0; It's good opportunity to eliminate this. You can use ASSERT_NO_EXCEPTION. > Source/WebCore/editing/Editor.h:39 > +#include "TextCheckingHelper.h" I really want hide this in .cpp file. > Source/WebCore/editing/Editor.h:408 > + void markAndReplaceFor(TextCheckingTypeMask, const Vector<TextCheckingResult>&, TextCheckingParagraph spellingParagraph, TextCheckingParagraph grammarParagraph); Is it possible to pass TextCheckingParagraph as a reference or a pointer?
Shinya Kawanaka
Comment 5 2011-12-05 15:47:39 PST
Created attachment 117950 [details] Patch
WebKit Review Bot
Comment 6 2011-12-06 02:32:13 PST
Comment on attachment 117950 [details] Patch Clearing flags on attachment: 117950 Committed r102111: <http://trac.webkit.org/changeset/102111>
WebKit Review Bot
Comment 7 2011-12-06 02:32:18 PST
All reviewed patches have been landed. Closing bug.
Vsevolod Vlasov
Comment 8 2011-12-06 03:39:28 PST
This patch broke compilation on several platforms. http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac%20Builder%20%28CG%29/builds/3684 ../editing/SpellingCorrectionController.h:58:1: error: struct 'TextCheckingResult' was previously declared as a class [-Werror,-Wmismatched-tags] struct TextCheckingResult; ^ ../editing/Editor.h:66:7: note: previous use is here class TextCheckingResult; ^ 1 error generated. Rolled out: <http://trac.webkit.org/changeset/102120> Reopening.
Shinya Kawanaka
Comment 9 2011-12-06 18:02:22 PST
Created attachment 118153 [details] Patch
WebKit Review Bot
Comment 10 2011-12-06 21:01:14 PST
Comment on attachment 118153 [details] Patch Clearing flags on attachment: 118153 Committed r102218: <http://trac.webkit.org/changeset/102218>
WebKit Review Bot
Comment 11 2011-12-06 21:01:19 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.