Bug 108475 - [Qt] Properly layer QtWebContext on top of the C API
Summary: [Qt] Properly layer QtWebContext on top of the C API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jocelyn Turcotte
URL:
Keywords:
Depends on: 109829
Blocks: 108471 109922 111435
  Show dependency tree / graph
 
Reported: 2013-01-31 06:16 PST by Jocelyn Turcotte
Modified: 2013-03-12 04:16 PDT (History)
4 users (show)

See Also:

Attachments
Patch (23.85 KB, patch)
2013-02-12 03:41 PST, Jocelyn Turcotte 		no flags Details | Formatted Diff | Diff
Patch (23.86 KB, patch)
2013-02-12 03:48 PST, Jocelyn Turcotte 		no flags Details | Formatted Diff | Diff
Patch (6.84 KB, patch)
2013-02-14 07:36 PST, Jocelyn Turcotte 		hausmann: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2013-01-31 06:16:45 PST 
QtWebContext, QtWebIconDatabaseClient and QtDownloadManager should be able to simply act as Qt bindings for the corresponding C APIs.
Comment 1 Jocelyn Turcotte 2013-02-12 03:41:15 PST 
Created attachment 187826 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-02-12 03:48:49 PST 
Created attachment 187827 [details]
Patch

Fix a sentence in the ChangeLog
Comment 3 Kenneth Rohde Christiansen 2013-02-12 05:04:43 PST 
Comment on attachment 187827 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:917
> +void QQuickWebViewPrivate::didReceiveMessageFromNavigatorQtObject(WKStringRef message)
>  {
>      QVariantMap variantMap;
> -    variantMap.insert(QLatin1String("data"), QString(message));
> +    variantMap.insert(QLatin1String("data"), WKStringCopyQString(message));

So your message is a WKString but WKStringCopyQString creates a QString? I think the naming of that method is confusing. CopyTo? anywya it that is the common naming then that is fine, I just found it a bit confusing
Comment 4 Jocelyn Turcotte 2013-02-13 02:21:42 PST 
Comment on attachment 187827 [details]
Patch

I'm working on splitting this patch in 2-3.
Comment 5 Jocelyn Turcotte 2013-02-14 07:36:25 PST 
Created attachment 188348 [details]
Patch

Splitted patch
Comment 6 Simon Hausmann 2013-02-20 07:01:02 PST 
Comment on attachment 188348 [details]
Patch

After the split-up this patch is indeed pretty simple now. LGTM. I'm hoping Benjamin can sign off on it :)
Comment 7 Benjamin Poulain 2013-02-21 15:44:20 PST 
Sorry, I missed it. Ping me if that happens again.

This looks great, I sign off on the change.
Comment 8 Jocelyn Turcotte 2013-03-12 04:16:59 PDT 
Committed r145516: <http://trac.webkit.org/changeset/145516>