Bug 68369 – [Qt][WK2] Add support for hover API in Qt WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Igor Trindade Oliveira]

Add support for hover API in Qt WebKit2

[Igor Trindade Oliveira]

Created attachment 108002 [details]
Patch.

Proposed implementation.

[Andreas Kling]

CC'ing some WK2 gentlemen since you're adding C API.

[Alexis Menard (darktears)]

Comment on attachment 108002 [details]
Patch.

Don't you miss some build systems here? GTK and the cmake stuff?

[Igor Trindade Oliveira]

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?

[Igor Trindade Oliveira]

Created attachment 108140 [details]
Patch

Updated patch.

[Anders Carlsson]

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.

[Anders Carlsson]

Also, this adds LGPL API headers. WebKit2 API headers should be BSD licensed.

[Caio Marcelo de Oliveira Filho]

Created attachment 108767 [details]
Patch

[Caio Marcelo de Oliveira Filho]

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.

[Alexis Menard (darktears)]

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

[Andreas Kling]

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.

[Caio Marcelo de Oliveira Filho]

Committed r96105: <http://trac.webkit.org/changeset/96105>