[Webkit-unassigned] [Bug 24953] Add automatic spell correction support in WebKit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24953


justin.garcia at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29450|review?                     |review+
               Flag|                            |




------- Comment #7 from justin.garcia at apple.com  2009-04-22 11:42 PDT -------
(From update of attachment 29450)
 #endif // Editor_h
+
+
+
Index: WebCore/editing/TypingCommand.cpp
=======

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

-    // 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)
-           
document()->frame()->editor()->markMisspellingsAfterTypingToPosition(p1);
+        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
better.

Other than that r=me


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.