[webkit-reviews] review denied: [Bug 29431] [Qt] REGRESSION:(r50665) QWebFrame::setScrollBarPolicy(Qt::Vertical, Qt::ScrollBarAlwaysOff) has no effect. : [Attachment 52641] patch_1 - v2.2 - attempt to build on Mac (same as "patch_1 - v2.1")

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Antonio Gomes (tonikitoo)
<tonikitoo at webkit.org>'s request for review:
Bug 29431: [Qt] REGRESSION:(r50665)
QWebFrame::setScrollBarPolicy(Qt::Vertical,Qt::ScrollBarAlwaysOff) has no
effect.
https://bugs.webkit.org/show_bug.cgi?id=29431

Attachment 52641: patch_1 - v2.2 - attempt to build on Mac (same as "patch_1 -
v2.1")
https://bugs.webkit.org/attachment.cgi?id=52641&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
(1) For the createView function, having two calls to set scrollbar modes is
problematic when each one results in an updateScrollers call underneath, e.g.,
on Mac.  You could just do:

frameView->setHorizontalLock(horizontalLock);
frameView->setVerticalLock(verticalLock);
frameView->setScrollbarModes(horizontalScrollbarMode, verticalScrollbarMode);

(2) Dump the lock argument from setHorizontalScrollbarMode and
setVerticalScrollbarMode. It's just inconsistent with setScrollbarModes, and
you've provided locking functions anyway (setHorizontalLock and
setVerticalLock).

(3) in qwebframe.cpp just do the lock call before setting the mode, e.g.,

d->frame->view()->setHorizontalLock(policy != Qt::ScrollBarAsNeeded);
d->frame->view()->setHorizontalScrollbarMode((ScrollbarMode)policy);

It's slightly more verbose, but I think it's nice to separate.