[Webkit-unassigned] [Bug 48385] Add WebKit SPI to scale a WebView

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48385





--- Comment #8 from Beth Dakin <bdakin at apple.com>  2010-10-27 14:00:14 PST ---
Thanks for the review, Darin! I thought I would post a new patch with some of the tweaks I made.

(In reply to comment #6)
> (From update of attachment 72044 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72044&action=review
> 
> > WebCore/page/Frame.cpp:163
> > +    , m_pageScaleFactor(1)
> 
> I’m disappointed to see more being added to Frame. I’ve been trying to cut down on operations in Frame, and would normally look for a different home for such a thing. On the other hand, I was unsuccessful at moving the page and text zoom factor out.
> 
> If this does have to be on the central “god object” maybe it makes more sense to put it on Page than on Frame.

This is the one change I did not make. I kept the scale factor on the Frame because zoom is there too. But I don't think there would be a problem moving it to Page, and you could convince me to do that.

> 
> > WebCore/page/Frame.cpp:967
> > +    if (m_pageScaleFactor == scale)
> > +        return;
> 
> Will this do the right thing in the case where the scale factor is the same, but the origin is different?
> 
> > WebCore/page/Frame.cpp:979
> > +    // Update the scroll position to the origin of the scale.
> > +    if (FrameView* view = this->view())
> > +        view->setScrollPosition(IntPoint(origin.x(), origin.y()));
> 
> I really don’t understand why this is the right thing to do and why we need a single function call that does both of these things? Can’t we set the scroll position another way.

I decided to get rid of the whole origin concept for the time being. We might want to add it back in later, but for now, I think we should make it the caller's problem after all. We can add new SPI or tweak this SPI to handle to origin if we decide it doesn't make sense for the caller.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.