Bug 77632

Summary: [Qt] Split QQuickWebViewPrivate in two classes, for desktop and touch behavior.
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, cmarcelo, hausmann, jesus, kenneth, menard, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77111    
Attachments:
Description Flags
Patch
none
Code moving patch (will squash before commit)
none
Patch kenneth: review+

Description Jocelyn Turcotte 2012-02-02 05:34:35 PST
[Qt] Split QQuickWebViewPrivate in two classes, for desktop and touch behavior.
Comment 1 Jocelyn Turcotte 2012-02-02 05:44:58 PST
Created attachment 125127 [details]
Patch
Comment 2 Jocelyn Turcotte 2012-02-02 05:51:39 PST
Created attachment 125128 [details]
Code moving patch (will squash before commit)
Comment 3 Kenneth Rohde Christiansen 2012-02-02 05:52:28 PST
Comment on attachment 125127 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Replace the runtime property setting the desktop behavior on each instance by a static boolean
> +        controlling the behavior used by WebView created in the future.
> +
> +        * UIProcess/API/qt/qquickwebpage.cpp:

a bit shorter lines and some added newlines could make this changelog readable

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:58
> +static bool sUseTraditionalDesktopBehaviour = false;

I believe s_ is what is commonly used

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:60
> +static QQuickWebViewPrivate* createPrivateObject(QQuickWebView* pub)

pub?

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:65
> +    if (sUseTraditionalDesktopBehaviour)
> +        return new QQuickWebViewDesktopPrivate(pub);
> +    return new QQuickWebViewTouchPrivate(pub);
> +}

im not convinced that Touch is the best way to describe the difference between the classes

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:684
> +void QQuickWebViewExperimental::setUseTraditionalDesktopBehaviour(bool enable)
> +{
> +    sUseTraditionalDesktopBehaviour = enable;
> +}
> +
> +bool QQuickWebViewExperimental::useTraditionalDesktopBehaviour()
> +{
> +    return sUseTraditionalDesktopBehaviour;
> +}
> +

So this cannot be changed at runtime?
Comment 4 Kenneth Rohde Christiansen 2012-02-02 06:56:14 PST
Comment on attachment 125127 [details]
Patch

Following ric discussions
Comment 5 Kenneth Rohde Christiansen 2012-02-02 06:56:25 PST
(In reply to comment #4)
> (From update of attachment 125127 [details])
> Following ric discussions

IRC that is
Comment 6 Jocelyn Turcotte 2012-02-02 08:00:25 PST
Created attachment 125133 [details]
Patch
Comment 7 Kenneth Rohde Christiansen 2012-02-02 11:18:48 PST
Comment on attachment 125133 [details]
Patch

rs=me
Comment 8 Jocelyn Turcotte 2012-02-03 06:49:56 PST
Committed r106658: <http://trac.webkit.org/changeset/106658>