[Webkit-unassigned] [Bug 51013] [Chromium] Should implement EditorClientImpl::requestCheckingOfString()

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from MORITA Hajime <morrita at google.com>  2010-12-15 21:51:53 PST ---
Hi Darin, thank you for reviewing!
I updated the patch. Could you take another look?

> > WebKit/chromium/public/WebTextCheckingResult.h:43
> > +    static WebTextCheckingResult misspellingAt(int location, int length);
> > +    static WebTextCheckingResult badGrammarAt(int location, int length);
> 
> these static methods need the WEBKIT_API prefix so that they can be exported from WebKit when it is built as a DLL.
Remove thsese, defining an enum instead.
> > WebKit/chromium/public/WebTextCheckingResult.h:46
> > +    int type() const { return m_type; }
> 
> it is generally better to define an enum for things like this.  then add code to
> AssertMatchingEnums.cpp to ensure that the API defined enums do not get out of sync
> with the WebCore defined ones.
Defined an enum. I didn't noticed AssertMatchingEnums.cpp. Thank you for pointing this.

> 
> > WebKit/chromium/public/WebTextCheckingResult.h:47
> > +    int location() const { return m_location; }
> 
> nit: i think it is more common to use the term "position" or "offset" in cases like this.
Renamed to position()

> 
> > WebKit/chromium/public/WebViewClient.h:177
> > +    // WebView::respondCheckingOfString()
> 
> nit: this comment is incorrect
Fixed.

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