[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 #94961|0                           |1
        is obsolete|                            |
  Attachment #94961|review?                     |
               Flag|                            |
  Attachment #95156|                            |review?
               Flag|                            |




--- Comment #12 from Hironori Bono <hbono at chromium.org>  2011-05-27 04:32:53 PST ---
Created an attachment (id=95156)
 --> (https://bugs.webkit.org/attachment.cgi?id=95156&action=review)
Patch v3 (only for Chromium)

Greetings Morita-san,

Thank you for your review and comments.
I have applied your comments and updated the change.

(In reply to comment #10)

> Hi Bono-san, thank you for update.
> You can add Skipped entries where other editing or spelling test is listed.
> It would reduce the chance of conflict.

Indeed. It is definitely better to avoid unnecessary conflicts in Skipped files.

> > Source/WebCore/dom/DocumentMarkerController.cpp:677
> > +        DocumentMarker marker = list->at(i);
> 
> Could you use const refrence if possible? It will save an extra copy.

Done. Thank you for noticing it.

> > Source/WebCore/dom/DocumentMarkerController.cpp:683
> > +        result->append(SpellcheckRange::create(marker.startOffset(), marker.endOffset() - marker.startOffset(), suggestions.release(), 0));
> 
> Could you add length() to DocumentMarker for here? It would be illustrative.

OK. I have added the length() method.

> > Source/WebCore/html/HTMLDivElement.idl:27
> > +        void addSpellcheckRange(in unsigned long start, in unsigned long length, in [Optional] DOMStringList suggestions, in [Optional] unsigned short options);
> 
> Don't we throw any exceptions? It might be a question about the spec though.

Even though the current API does not send exceptions, we will hear opinions of users with actual implementation and clearify such ambiguous issues.

> > LayoutTests/ChangeLog:6
> > +        https://bugs.webkit.org/show_bug.cgi?id=59693
> 
> Can we have a test pixel test for that to ensure that the user-marker is actually visible?

OK. I have added another test. (I prefer script tests, though.)

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.