Bug 112261 - [WK2][EFL] Mark not implemented methods in TextCheckerEfl
Summary: [WK2][EFL] Mark not implemented methods in TextCheckerEfl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Grzegorz Czajkowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-13 08:04 PDT by Grzegorz Czajkowski
Modified: 2013-04-09 00:29 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch (3.87 KB, patch)
2013-03-13 08:07 PDT, Grzegorz Czajkowski
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>