[webkit-reviews] review granted: [Bug 24953] Add automatic spell correction support in WebKit : [Attachment 29450] Support for automatic spelling corrections

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 22 11:42:24 PDT 2009

Justin Garcia <justin.garcia at apple.com> has granted Siddhartha Chattopadhyay
<sidchat at google.com>'s request for review:
Bug 24953: Add automatic spell correction support in WebKit

Attachment 29450: Support for automatic spelling corrections

------- Additional Comments from Justin Garcia <justin.garcia at apple.com>
 #endif // Editor_h
Index: WebCore/editing/TypingCommand.cpp

Looks like you accidently added some unnecessary newlines to the end of this

-    // If the selection starts inside a table, just insert the paragraph
separator normally
-    // Breaking the blockquote would also break apart the table, which is
unecessary when inserting a newline
-    if (enclosingNodeOfType(endingSelection().start(), &isTableStructureNode))
-	 insertParagraphSeparator();
-	 return;
-    }

Please remove this before you check in.  If it's a workaround for a bug you're
seeing please file it.

-	 if (p1 != p2)
+	 if (p1 != p2) {
+	     RefPtr<Range> misspellingRange;
+	     Frame* frame = document()->frame();

Would be nice if you turned this into an early return to avoid the extra level
of if-nesting.

+	     // Autocorrect the misspelled word.
+	     if (misspellingRange.get() != NULL) {
+		 // Get the misspelled word.

In WebCore the convention is to use != 0, and you don't need the .get() here. 
And again, you could turn this into an early return.

frame->editor()->replaceSelectionWithText(autocorrectedString, false, true);

You actually don't need smart replace here, but it won't matter because smart
replace will only do anything if the selection being replaced is between is
flanked by non-whitespace, which isn't the case when replacing a word.	Your
use of it makes me think that smart replace needs to be renamed or commented

Other than that r=me

More information about the webkit-reviews mailing list