[Webkit-unassigned] [Bug 90927] [EFL][WK2] Add ewk_view_find_client.h / cpp for wrapping WKPageSetPageFindClient.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 2 10:53:31 PDT 2012


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


Raphael Kubo da Costa (rakuco) <rakuco at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #155996|                            |commit-queue-
               Flag|                            |




--- Comment #36 from Raphael Kubo da Costa (rakuco) <rakuco at webkit.org>  2012-08-02 10:53:28 PST ---
(From update of attachment 155996)
View in context: https://bugs.webkit.org/attachment.cgi?id=155996&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:62
> + * - "text,found", unsigned*: the requested text was found and it gives the number of matches.

Using only "unsigned" without an actual type after that is a WebKit convention not followed by the EFL themselves. You should either report that it's an unsigned int, unsigned long or whatever or, if you think it makes sense, change that to an actual struct, which could contain the text searched and the match count.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:224
> +typedef enum _EwkFindOptions {

Although it does not make much difference, in WK1 we have been declaring enums this way:
  enum _Foo_Bar {
  };
  typedef enum _Foo_Bar Foo_Bar;

The EFL themselves are not very consistent in this regard, so I recommend following our usual conventions here.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:234
> +} EwkFindOptions;

structs and enums have each word separated by an underscore, so this should actually be called Ewk_Find_Options.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:528
> +* Clears the highlight of searched text.

If "WKPageHideFindUI" is only responsible for clearing the search highlights in the page, first of all ewk_view_text_find should document that the highlight happens at all; if that API call is responsible for more than that, the documentation here should be updated accordingly.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:534
> +EAPI Eina_Bool ewk_view_text_find_finish(Evas_Object *o);

"finish" sounds vague in this context. A good name would really depend on what this call really does; if it only clears the search highlights, "ewk_view_text_find_highlight_clear()" would be more self-explanatory.

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