[Webkit-unassigned] [Bug 59693] [Feature Request] Need SpellCheck API

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from MORITA Hajime <morrita at google.com>  2011-05-25 05:18:10 PST ---
(From update of attachment 94769)
View in context: https://bugs.webkit.org/attachment.cgi?id=94769&action=review

Bono-san, thank you for doing this!
I did some nit picking. The overall shape looks good for me.
But I'd like to hear from other interested folks about its overall design.

> Source/WebCore/dom/DocumentMarker.h:63
> +#if ENABLE(SPELLCHECK_API)

This adds an extra "," for the enums. I think You can just add UserSpelling without ENABLE() guard.
It's never used in disabled case and there is no runtime overhead.
Auto-correction related enums do same thing.

> Source/WebCore/dom/DocumentMarkerController.cpp:686
> +    return result.release();

You don't need release()

> Source/WebCore/dom/DocumentMarkerController.cpp:689
> +bool DocumentMarkerController::addUserSpellingMarker(Node* node, unsigned start, int length, RefPtr<DOMStringList> suggestions, int options)

Can length be unsigned?

> Source/WebCore/dom/DocumentMarkerController.cpp:721
> +Node* DocumentMarkerController::userSpellingNode(Node* node) const

We can define this as a static function.

> Source/WebCore/html/SpellcheckRange.h:40
> +    static PassRefPtr<SpellcheckRange> create() { return adoptRef(new SpellcheckRange()); }

Inline functions should be defined after the class definition and class definition should be function-body free.
This isn't mandatory. But it's a recent preference.

> Source/WebCore/html/SpellcheckRangeList.h:31
> +#include "SpellcheckRange.h"

can we replace this with a forward declaration?

> Source/WebCore/rendering/InlineTextBox.cpp:961
> +#if ENABLE(SPELLCHECK_API)

We don't need these guards

> Source/WebKit/chromium/src/WebRuntimeFeatures.cpp:341
> +#if ENABLE(SPELLCHECK_API)

Please add UNUSED_PARAM() for disabled path.

> LayoutTests/ChangeLog:1
> +2011-05-25  Hironori Bono  <hbono at chromium.org>

Please update skipped lists for unsupported platforms.

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