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

[bugzilla-daemon at webkit.org]

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


Hironori Bono <hbono at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #94769|0                           |1
        is obsolete|                            |
  Attachment #94769|review?                     |
               Flag|                            |
  Attachment #94961|                            |review?
               Flag|                            |




--- Comment #9 from Hironori Bono <hbono at chromium.org>  2011-05-26 05:30:04 PST ---
Created an attachment (id=94961)
 --> (https://bugs.webkit.org/attachment.cgi?id=94961&action=review)
Patch v2 (only for Chromium)

(In reply to comment #8)

Thank you for your review and comments. Even though I have not applied your comments totally, I have uploaded the updated change because I have uploaded an old version yesterday.

> > 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.

Thank you for your suggestion. I have removed the guard here and added a comma at the end of this line.

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

Ah, yes. It is better to use unsigned since the callers use unsigned.

> > Source/WebCore/html/SpellcheckRangeList.h:31
> > +#include "SpellcheckRange.h"
> 
> can we replace this with a forward declaration?

Thank you for your catch. Yes, we can replace it. I have replaced this include with a forward declaration.

> > Source/WebCore/rendering/InlineTextBox.cpp:961
> > +#if ENABLE(SPELLCHECK_API)
> 
> We don't need these guards

Thank you for your comment. I have removed them.

> > Source/WebKit/chromium/src/WebRuntimeFeatures.cpp:341
> > +#if ENABLE(SPELLCHECK_API)
> 
> Please add UNUSED_PARAM() for disabled path.

Indeed. I forgot adding UNUSED_PARAM() here.

> > LayoutTests/ChangeLog:1
> > +2011-05-25  Hironori Bono  <hbono at chromium.org>
> 
> Please update skipped lists for unsupported platforms.

Ok. I have added my new test to Skipped files.

Regards,

Hironori Bono

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