[Webkit-unassigned] [Bug 74071] [Chromium] Chromium should have EditorClientImpl::checkTextOfParagraph

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 13 16:00:44 PST 2011


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





--- Comment #8 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2011-12-13 16:00:43 PST ---
(In reply to comment #7)
> > By the way, it seems like WebSpellCheckClient is misnamed.  It should probably
> > be called WebTextChecker.  Such a renaming is out-of-scope for this patch, but
> > I just wanted to call attention to the issue.
> 
> I'll do this another patch if necessary.

OK

> > This appears to run synchronously, like spellCheck.  Should we have an asynchronous
> > version like requestCheckingOfText?  Also, what about the optionalSuggestions out-
> > param of spellCheck?  You don't need a way to provide suggestions with this new API?
> 
> I have a plan to add asynchronous version after this patch is landed.
> I want to have the same interface WebCore::TextCheckerClient::checkTextOfParagraph. So optional suggestions is not mandatory. But we can specify adding auto correction list by WebTextCheckingTypeMask.

OK


> > > Source/WebKit/chromium/public/WebTextChecking.h:36
> > > +enum WebTextCheckingType {
> > 
> > WebTextCheckingResult has a similar enum.  Presumably, that one should be deleted
> > in favor of this one?
> 
> Yeah, I want to keep WebTextCheckingType. So the previous enum should be removed. But removing it this time will break chromium build, so after this patch is landed, I'll change chromium code, then come back to WebKit to remove it.

OK


> > > Source/WebKit/chromium/src/EditorClientImpl.cpp:795
> > > +    m_webView->spellCheckClient()->checkTextOfParagraph(WebString(text, length), webMask, &webResults);
> > 
> > do we need to concern ourselves with the cost of constructing the WebString here?
> > it seems like if we are doing a lot of spell checking that we might prefer to
> > avoid the heap allocation here.  have you measured this to see if it matters?
> 
> Hmm... Currently SpellCheckClient uses WebString interface instead of UChar* and int. If it is important, we should change all the interface of SpellCheckClient, I guess. Currently it does not matter, I guess.

OK... perhaps this is just something worth investigating to see if it matters.
does not have to block this patch.

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



More information about the webkit-unassigned mailing list