[webkit-reviews] review granted: [Bug 72508] [Qt][WK2] Do not apply new viewport properties until after the first visually non-empty layout. : [Attachment 115381] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 16 08:24:28 PST 2011
Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Zalan Bujtas
<zbujtas at gmail.com>'s request for review:
Bug 72508: [Qt][WK2] Do not apply new viewport properties until after the first
visually non-empty layout.
https://bugs.webkit.org/show_bug.cgi?id=72508
Attachment 115381: Patch
https://bugs.webkit.org/attachment.cgi?id=115381&action=review
------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115381&action=review
> Source/WebKit2/ChangeLog:6
> + Delay applying viewport properties on the new document until after
the first visually
not on the new document but on our viewport item
> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:109
> void QQuickWebViewPrivate::loadDidCommit()
> {
> - if (!useTraditionalDesktopBehaviour)
> - interactionEngine->reset();
> + transitioningToNewPage = true;
> }
Maybe we should set this to true on crash as well.
> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:126
> + // Do not apply the new content size, until after the first visually
non-empty layout finished.
I dont think the comment is needed
> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:143
> + // Make sure, the new viewport values are not applied to the old
content.
same here
> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:190
> _q_viewportUpdated();
I hate callbacks like this here... have to look what it does... not your fault
though :-) and not part of this patch
> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:88
> + class PostTransitionState {
A comment here would make sense, linking to the wiki page for instance
> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:97
> +
p->interactionEngine->applyConstraints(p->computeViewportConstraints());
future: Maybe those constraints shouldnt be in the interaction engine itself
> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:111
> + bool isTransitioningToNewPage() const {return transitioningToNewPage; }
missing space
> Source/WebKit2/UIProcess/qt/ClientImpl.cpp:157
> + if (!WKFrameIsMainFrame(frame))
> + return;
I prefer a newline after this
More information about the webkit-reviews
mailing list