[webkit-reviews] review granted: [Bug 49423] Regression: contraction is considered misspelling. : [Attachment 73806] Proposed patch (v3)

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted jpu at apple.com's request for review:
Bug 49423: Regression: contraction is considered misspelling.
https://bugs.webkit.org/show_bug.cgi?id=49423

Attachment 73806: Proposed patch (v3)
https://bugs.webkit.org/attachment.cgi?id=73806&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=73806&action=review

> WebCore/ChangeLog:13
> +	   (WebCore::isAmbiguousBoundaryCharacter): Moved function to the top
of the file so that it.

So that it what? :)

> WebCore/ChangeLog:20
> +	     is not a partial contraction, such as "wouldn'", before mark it as
misspelled. Also update

update what?

> WebCore/editing/Editor.cpp:2021
> +void Editor::markMisspellingsAfterTypingToWord(const VisiblePosition
&wordStart, const VisibleSelection& selectionAfterTyping)

Can we ASSERT_ARG here that selectionAfterTyping is a caret selection?

> WebCore/editing/Editor.cpp:2057
> +    VisibleSelection adjacentWords = VisibleSelection(startOfWord(wordStart,
LeftWordIfOnBoundary), endOfWord(wordStart, RightWordIfOnBoundary));

You don’t need to call startOfWord() on wordStart, because the only caller
passes a word start already. Maybe this should be ASSERT_ARG()ed right at the
beginning. Anyway, your patch just made this obvious by improving the naming.
You don’t need to change anything now.

> WebCore/editing/Editor.cpp:2071
> +    markMisspellings(VisibleSelection(startOfWord(wordStart,
LeftWordIfOnBoundary), endOfWord(wordStart, RightWordIfOnBoundary)),
misspellingRange);

Ditto.