Bug 112261

Summary:

[WK2][EFL] Mark not implemented methods in TextCheckerEfl

Product:

WebKit

Reporter:

Grzegorz Czajkowski <g.czajkowski>

Component:

WebKit EFL

Assignee:

Grzegorz Czajkowski <g.czajkowski>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, gyuyoung.kim, kenneth, lucas.de.marchi, mikhail.pozdnyakov, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch	andersca: review+

Description Grzegorz Czajkowski 2013-03-13 08:04:04 PDT

As a continuation of bug 111713 I'd like to prose further improvements of EFL's TextChecker.

It's no need to call client's methods in TextCheckerEfl if they are not implemented.
At the moment, they are using internal WK2 WebTextChecker class wich is not recommended.
It's better to mark them as not implemented unless we provide proper implementation.

Comment 1 Grzegorz Czajkowski 2013-03-13 08:07:22 PDT

Created attachment 192924 [details]
proposed patch

Comment 2 Mikhail Pozdnyakov 2013-03-13 08:13:46 PDT

Comment on attachment 192924 [details]
proposed patch

Makes sense.

Comment 3 Mikhail Pozdnyakov 2013-03-13 08:15:48 PDT

(In reply to comment #2)
> (From update of attachment 192924 [details])
> Makes sense.
The class TextChecker is WK2 generic so we cannot just remove those methods, right?

Comment 4 Grzegorz Czajkowski 2013-03-13 08:20:36 PDT

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 192924 [details] [details])
> > Makes sense.
> The class TextChecker is WK2 generic so we cannot just remove those methods, right?

Exactly, thanks for the review!

Comment 5 Gyuyoung Kim 2013-03-13 20:55:55 PDT

Comment on attachment 192924 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192924&action=review

Should we remove this at the moment ? Is it better to remove when we prepare a patch for new implementation ?

> Source/WebKit2/ChangeLog:10
> +        class wich is not recommended.

Typo: wich ?

Comment 6 Grzegorz Czajkowski 2013-03-14 00:53:05 PDT

(In reply to comment #5)
> (From update of attachment 192924 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192924&action=review
> 
> Should we remove this at the moment ? Is it better to remove when we prepare a patch for new implementation ?

IMO, it's better to remove it. Grammar checking implementation is more complicated than the spell checking due to libraries/services unavailability. We are not sure whether the implementation will be provided.

Secondly our text checker doesn't implement them (the client's callbacks are not initialized) so it doesn't make sense to invoke them.

Finally, Kenneth mentioned that we shouldn't call the client's methods in TextCheckerEfl as the matter of fact we use an internal WebKit implementation (with Enchant support). It'd be better to call them directly as Mac is doing in their TextCheckerMac.mm

> 
> > Source/WebKit2/ChangeLog:10
> > +        class wich is not recommended.
> 
> Typo: wich ?

Thanks. If you don't mind I fix it before landing.

Comment 7 Gyuyoung Kim 2013-03-14 01:06:35 PDT

Ok, looks make sense.

Comment 8 Grzegorz Czajkowski 2013-04-09 00:29:16 PDT

Committed r147993: <http://trac.webkit.org/changeset/147993>