RESOLVED FIXED 108920
[Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
https://bugs.webkit.org/show_bug.cgi?id=108920
Summary [Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
Simon Hausmann
Reported 2013-02-05 00:47:40 PST
[Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
Attachments
Patch (10.06 KB, patch)
2013-02-05 00:50 PST, Simon Hausmann
jturcotte: review+
Simon Hausmann
Comment 1 2013-02-05 00:50:11 PST
Jocelyn Turcotte
Comment 2 2013-02-05 06:15:23 PST
Comment on attachment 186567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186567&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:327 > + { > + WKPageFindClient findClient; > + memset(&findClient, 0, sizeof(WKPageFindClient)); > + findClient.version = kWKPageFindClientCurrentVersion; > + findClient.clientInfo = this; > + findClient.didFindString = didFindString; > + findClient.didFailToFindString = didFailToFindString; > + WKPageSetPageFindClient(toAPI(webPageProxy.get()), &findClient); > + } If the intent is to do so with most C API clients, it would be cleaner move all their initialization in a separate method. We can also do this once we have more of them here. r=me either way.
Simon Hausmann
Comment 3 2013-02-05 07:24:52 PST
This still needs a sign-off from a WK2 owner. Benjamin, can you help us with this? :)
Benjamin Poulain
Comment 4 2013-02-05 17:29:24 PST
Comment on attachment 186567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186567&action=review Okay with me, feel free to land. Isn't that exposing a bit too much in case you want to make QQuickWebView publicly available from C++? > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:2082 > + didFindString(page, string, 0, clientInfo); Maybe prefix by QQuickWebViewPrivate:: for clarity?
Simon Hausmann
Comment 5 2013-02-07 00:25:18 PST
(In reply to comment #4) > (From update of attachment 186567 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186567&action=review > > Okay with me, feel free to land. Thank you! > Isn't that exposing a bit too much in case you want to make QQuickWebView publicly available from C++? It is becoming less and less likely that we're going to make the QML implementation a public C++ API :) > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:2082 > > + didFindString(page, string, 0, clientInfo); > > Maybe prefix by QQuickWebViewPrivate:: for clarity? Good idea.
Simon Hausmann
Comment 6 2013-02-07 00:39:57 PST
Note You need to log in before you can comment on or make changes to this bug.