Bug 68369 - [Qt][WK2] Add support for hover API in Qt WebKit2
Summary: [Qt][WK2] Add support for hover API in Qt WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P3 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords: Qt, QtTriaged
Depends on: 68426
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-19 10:13 PDT by Igor Trindade Oliveira
Modified: 2011-09-27 07:09 PDT (History)
6 users (show)

See Also:


Attachments
Patch. (22.63 KB, patch)
2011-09-20 08:00 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (23.71 KB, patch)
2011-09-21 06:14 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (14.40 KB, patch)
2011-09-26 17:51 PDT, Caio Marcelo de Oliveira Filho
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Trindade Oliveira 2011-09-19 10:13:11 PDT
Add support for hover API in Qt WebKit2
Comment 1 Igor Trindade Oliveira 2011-09-20 08:00:11 PDT
Created attachment 108002 [details]
Patch.

Proposed implementation.
Comment 2 Andreas Kling 2011-09-21 04:39:56 PDT
CC'ing some WK2 gentlemen since you're adding C API.
Comment 3 Alexis Menard (darktears) 2011-09-21 04:54:49 PDT
Comment on attachment 108002 [details]
Patch.

Don't you miss some build systems here? GTK and the cmake stuff?
Comment 4 Igor Trindade Oliveira 2011-09-21 06:13:30 PDT
CMake code is in the beginning of the patch and GTK is handled by GNUmakefile.am.

(In reply to comment #3)
> (From update of attachment 108002 [details])
> Don't you miss some build systems here? GTK and the cmake stuff?
Comment 5 Igor Trindade Oliveira 2011-09-21 06:14:28 PDT
Created attachment 108140 [details]
Patch

Updated patch.
Comment 6 Anders Carlsson 2011-09-21 10:36:55 PDT
Comment on attachment 108140 [details]
Patch

The API added doesn't follow the naming conventions we use for returning copies. Also, https://bugs.webkit.org/show_bug.cgi?id=68426 also adds the same API.
Comment 7 Anders Carlsson 2011-09-21 12:16:50 PDT
Also, this adds LGPL API headers. WebKit2 API headers should be BSD licensed.
Comment 8 Caio Marcelo de Oliveira Filho 2011-09-26 17:51:56 PDT
Created attachment 108767 [details]
Patch
Comment 9 Caio Marcelo de Oliveira Filho 2011-09-26 17:55:40 PDT
Igor went for vacations, so I'm picking up this. WKHitTestResult landed already for another bug.

Besides removing the WKHitTestResult, this patch also
- change MiniBrowser to use the new feature;
- add tests for the signal;
- remove the third parameter textContent, since I'm not convinced it is necessary.
Comment 10 Alexis Menard (darktears) 2011-09-26 18:40:41 PDT
Comment on attachment 108767 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:91
> +    void linkHovered(const QUrl& url, const QString &title);

Style issue. const QString&.
Comment 11 Andreas Kling 2011-09-27 06:27:27 PDT
Comment on attachment 108767 [details]
Patch

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

r=me

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:436
> +    if (linkURL != lastHoveredURL || linkTitle != lastHoveredTitle) {

I would write this as:
if (linkURL == lastHoveredURL && linkTitle == lastHoveredTitle)
    return;

> Source/WebKit2/UIProcess/qt/ClientImpl.cpp:167
> +void qt_wk_mouseDidMoveOverElement(WKPageRef page, WKHitTestResultRef hitTestResult, WKEventModifiers modifiers, WKTypeRef userData, const void *clientInfo)

Style, const void* clientInfo.

> Source/WebKit2/UIProcess/qt/ClientImpl.h:44
> +void qt_wk_mouseDidMoveOverElement(WKPageRef, WKHitTestResultRef, WKEventModifiers, WKTypeRef, const void *clientInfo);

Ditto.
Comment 12 Caio Marcelo de Oliveira Filho 2011-09-27 07:09:22 PDT
Committed r96105: <http://trac.webkit.org/changeset/96105>