[Webkit-unassigned] [Bug 96481] [Qt][WK2] Add API to find text from page

[bugzilla-daemon at webkit.org]

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


Jocelyn Turcotte <jocelyn.turcotte at digia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #164346|review?                     |review-
               Flag|                            |




--- Comment #14 from Jocelyn Turcotte <jocelyn.turcotte at digia.com>  2012-11-09 01:03:53 PST ---
(From update of attachment 164346)
View in context: https://bugs.webkit.org/attachment.cgi?id=164346&action=review

It's a pretty good patch overall at this point, here are some minor issues.

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:392
> +    void textFound(unsigned matchCount);

I don't think we use unsigned so much in APIs, I'd rather have it exposed as a signed int for consistency.

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:85
> +    virtual void didFindString(unsigned matchCount);

No need to make it virtual if it won't be overridden by QQuickWebView(Legacy|Flickable)Private.

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:73
> +        function toggle() {

Could you also trigger it with the ctrl+f shortcut in MiniBrowserApplication::notify?
Having the TextInput get/lose focus when toggle() is called would be awesome too.

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:94
> +        MouseArea {

Please add a comment about its purpose.

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:151
> +            Rectangle {
> +                id: prevButton

We really have to pull this common code out in a ToolbarButton.qml as this code is getting quite duplicated, but this can be done in a different patch if you don't want to take care of it.

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