[webkit-reviews] review granted: [Bug 59693] [Feature Request] Need SpellCheck API : [Attachment 95413] Patch v4 (only for Chromium)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue May 31 21:19:56 PDT 2011
Brent Fulgham <bfulgham at webkit.org> has granted Hironori Bono
<hbono at chromium.org>'s request for review:
Bug 59693: [Feature Request] Need SpellCheck API
https://bugs.webkit.org/show_bug.cgi?id=59693
Attachment 95413: Patch v4 (only for Chromium)
https://bugs.webkit.org/attachment.cgi?id=95413&action=review
------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95413&action=review
What a great, complete, patch! This looks like it would be easy to implement
under Windows (and Mac OS X). Once this initial patch lands I can help get it
working under Windows and/or Mac if you would like.
r+, but please correct the couple of minor comments before landing.
> Source/WebCore/dom/DocumentMarker.h:64
> + // this marker is almost identical as the above Spelling marker, we
need
... almost identical TO the above Spelling marker ...
> Source/WebCore/dom/DocumentMarker.h:105
> + int length() const { return m_endOffset - m_startOffset; }
Is this ever allowed to be negative? Should this also be unsigned?
> Source/WebCore/dom/DocumentMarkerController.cpp:665
> + return result.release();
No need for release here.
> Source/WebCore/dom/DocumentMarkerController.cpp:679
> + return result.release();
Ditto
> Source/WebCore/html/SpellcheckRange.h:31
> +#include "DOMStringList.h"
Can this be a forward declaration? I can't remember if PassRefPtr needs the
full implementation.
More information about the webkit-reviews
mailing list